* [PATCH] MIPS: dec: Avoid la pseudo-instruction in delay slots
@ 2016-09-02 14:19 Paul Burton
2016-09-19 22:30 ` Maciej W. Rozycki
0 siblings, 1 reply; 4+ messages in thread
From: Paul Burton @ 2016-09-02 14:19 UTC (permalink / raw)
To: linux-mips; +Cc: Paul Burton, Maciej W. Rozycki, Ralf Baechle, linux-kernel
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.
Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---
arch/mips/dec/int-handler.S | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/mips/dec/int-handler.S b/arch/mips/dec/int-handler.S
index d7b9918..54ddca1 100644
--- a/arch/mips/dec/int-handler.S
+++ b/arch/mips/dec/int-handler.S
@@ -137,16 +137,16 @@
and t0,t1 # isolate allowed ones
beqz t0,spurious
-
#ifdef CONFIG_32BIT
and t2,t0
bnez t2,fpu # handle FPU immediately
#endif
+ nop
/*
* Find irq with highest priority
*/
- PTR_LA t1,cpu_mask_nr_tbl
+ PTR_LA t1,cpu_mask_nr_tbl
1: lw t2,(t1)
nop
and t2,t0
@@ -191,11 +191,12 @@
1: and t0,t1 # mask out allowed ones
beqz t0,spurious
+ nop
/*
* Find irq with highest priority
*/
- PTR_LA t1,asic_mask_nr_tbl
+ PTR_LA t1,asic_mask_nr_tbl
2: lw t2,(t1)
nop
and t2,t0
--
2.9.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] MIPS: dec: Avoid la pseudo-instruction in delay slots
2016-09-02 14:19 [PATCH] MIPS: dec: Avoid la pseudo-instruction in delay slots Paul Burton
@ 2016-09-19 22:30 ` Maciej W. Rozycki
2016-09-20 12:33 ` Ralf Baechle
0 siblings, 1 reply; 4+ messages in thread
From: Maciej W. Rozycki @ 2016-09-19 22:30 UTC (permalink / raw)
To: Paul Burton; +Cc: linux-mips, Ralf Baechle, linux-kernel
On Fri, 2 Sep 2016, Paul Burton 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.
Maciej
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MIPS: dec: Avoid la pseudo-instruction in delay slots
2016-09-19 22:30 ` Maciej W. Rozycki
@ 2016-09-20 12:33 ` Ralf Baechle
2016-10-03 0:21 ` Maciej W. Rozycki
0 siblings, 1 reply; 4+ messages in thread
From: Ralf Baechle @ 2016-09-20 12:33 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Paul Burton, linux-mips, linux-kernel
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] MIPS: dec: Avoid la pseudo-instruction in delay slots
2016-09-20 12:33 ` Ralf Baechle
@ 2016-10-03 0:21 ` Maciej W. Rozycki
0 siblings, 0 replies; 4+ messages in thread
From: Maciej W. Rozycki @ 2016-10-03 0:21 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Paul Burton, linux-mips, linux-kernel
On Tue, 20 Sep 2016, Ralf Baechle wrote:
> > 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 ;-)
Well, the GCC driver already handles it and catches warnings from GAS if
requested, doesn't it?
> 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.
That's what I noted above and as also noted you only need the %hi/%lo
variant due to the `-msym32' restriction for 64-bit DECstation kernels;
the kernel is always linked to KSEG0.
Alternatively for maximum safety perhaps we could define PTR_LA_EXPLICIT
or suchlike with a suitable open-coded sequence for the pointer width
chosen; note however that `-msym32' doesn't set any CPP's internal
predefined macro, so we'd have to define our own to control this.
> Maciej, can you test this?
I'll see what I can do; I've been travelling a lot lately.
Maciej
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-10-03 0:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-02 14:19 [PATCH] MIPS: dec: Avoid la pseudo-instruction in delay slots Paul Burton
2016-09-19 22:30 ` Maciej W. Rozycki
2016-09-20 12:33 ` Ralf Baechle
2016-10-03 0:21 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).