All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ralf Baechle <ralf@linux-mips.org>
To: "Maciej W. Rozycki" <macro@linux-mips.org>
Cc: Paul Burton <paul.burton@imgtec.com>,
	linux-mips@linux-mips.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] MIPS: dec: Avoid la pseudo-instruction in delay slots
Date: Tue, 20 Sep 2016 14:33:01 +0200	[thread overview]
Message-ID: <20160920123301.GN5881@linux-mips.org> (raw)
In-Reply-To: <alpine.LFD.2.20.1609192323450.19345@eddie.linux-mips.org>

On Mon, Sep 19, 2016 at 11:30:25PM +0100, Maciej W. Rozycki wrote:

> > When expanding the la or dla pseudo-instruction in a delay slot the GNU
> > assembler will complain should the pseudo-instruction expand to multiple
> > actual instructions, since only the first of them will be in the delay
> > slot leading to the pseudo-instruction being only partially executed if
> > the branch is taken. Use of PTR_LA in the dec int-handler.S leads to
> > such warnings:
> > 
> >   arch/mips/dec/int-handler.S: Assembler messages:
> >   arch/mips/dec/int-handler.S:149: Warning: macro instruction expanded into multiple instructions in a branch delay slot
> >   arch/mips/dec/int-handler.S:198: Warning: macro instruction expanded into multiple instructions in a branch delay slot
> > 
> > Avoid this by placing nops in the delay slots of the affected branches,
> > leading to the PTR_LA macros being placed after the branches & their
> > delay slots. Although the nop isn't strictly needed, it's an
> > insignificant cost & satisfies the assembler easily with more
> > readable code than the possible alternative of manually expanding the
> > la/dla pseudo-instructions & placing the appropriate first instruction
> > into the delay slots.
> 
>  I take it it's a quest for a clean compilation with no warnings reported, 
> as the message is otherwise harmless and correct code is produced here.
> 
>  Some of the systems affected are not necessarily fast (e.g. clocked at 
> 12MHz), so I wonder if we shouldn't just bite the bullet and expand the 
> %hi/%lo pair here.  Even with R4k DECstations we only support `-msym32' 
> compilation only, due to R4k errata, so it's not like the higher parts 
> will ever be needed for address calculation.
> 
>  Alternatively we could have a `.set warn'/`.set nowarn' setting in GAS, 
> previously discussed I believe, although it would take a bit to propagate.

Yeah, I'm already looking into the other direction whenever this warning
pops up - and it does so often.  I've even pondered submitting something
like -Werror for gas ;-)

It's not very elegant but you could just open code everything, see below
patch.  Compiles but not runtime tested due to lack of hardware.

Maciej, can you test this?

  Ralf

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

 arch/mips/dec/int-handler.S | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/arch/mips/dec/int-handler.S b/arch/mips/dec/int-handler.S
index d7b9918..20035fc 100644
--- a/arch/mips/dec/int-handler.S
+++ b/arch/mips/dec/int-handler.S
@@ -146,7 +146,25 @@
 		/*
 		 * Find irq with highest priority
 		 */
-		 PTR_LA	t1,cpu_mask_nr_tbl
+		# open coded PTR_LA t1, cpu_mask_nr_tbl
+#if (_MIPS_SZPTR == 32)
+		# open coded la t1, cpu_mask_nr_tbl
+		lui	t1, %hi(cpu_mask_nr_tbl)
+		addiu	t1, %lo(cpu_mask_nr_tbl)
+		
+#endif
+#if (_MIPS_SZPTR == 64)
+		# open coded dla t1, cpu_mask_nr_tbl
+		.set	push
+		.set	noat
+		lui	t1, %highest(cpu_mask_nr_tbl)
+		lui	AT, %hi(cpu_mask_nr_tbl)
+		daddiu	t1, t1, %higher(cpu_mask_nr_tbl)
+		daddiu	AT, AT, %lo(cpu_mask_nr_tbl)
+		dsll	t1, 32
+		daddu	t1, t1, AT
+		.set	pop
+#endif
 1:		lw	t2,(t1)
 		nop
 		and	t2,t0
@@ -195,7 +213,25 @@
 		/*
 		 * Find irq with highest priority
 		 */
-		 PTR_LA	t1,asic_mask_nr_tbl
+		# open coded PTR_LA t1,asic_mask_nr_tbl
+#if (_MIPS_SZPTR == 32)
+		# open coded la t1, asic_mask_nr_tbl
+		lui	t1, %hi(asic_mask_nr_tbl)
+		addiu	t1, %lo(asic_mask_nr_tbl)
+		
+#endif
+#if (_MIPS_SZPTR == 64)
+		# open coded dla t1, asic_mask_nr_tbl
+		.set	push
+		.set	noat
+		lui	t1, %highest(asic_mask_nr_tbl)
+		lui	AT, %hi(asic_mask_nr_tbl)
+		daddiu	t1, t1, %higher(asic_mask_nr_tbl)
+		daddiu	AT, AT, %lo(asic_mask_nr_tbl)
+		dsll	t1, 32
+		daddu	t1, t1, AT
+		.set	pop
+#endif
 2:		lw	t2,(t1)
 		nop
 		and	t2,t0

  reply	other threads:[~2016-09-20 12:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-02 14:19 [PATCH] MIPS: dec: Avoid la pseudo-instruction in delay slots Paul Burton
2016-09-02 14:19 ` Paul Burton
2016-09-19 22:30 ` Maciej W. Rozycki
2016-09-20 12:33   ` Ralf Baechle [this message]
2016-10-03  0:21     ` Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160920123301.GN5881@linux-mips.org \
    --to=ralf@linux-mips.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=macro@linux-mips.org \
    --cc=paul.burton@imgtec.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.