* [RESEND PATCH v1] MIPS: uasm: false warning on use of uasm_i_lui()
@ 2020-09-08 16:45 Jim Quinlan
2020-09-10 9:37 ` Thomas Bogendoerfer
0 siblings, 1 reply; 3+ messages in thread
From: Jim Quinlan @ 2020-09-08 16:45 UTC (permalink / raw)
To: linux-mips, bcm-kernel-feedback-list, james.quinlan
Cc: Thomas Bogendoerfer, open list
Currently, the example uasm code
uasm_i_lui(p, tmp, 0xa000);
issues a warning at Linux boot when the code is "assembled". This is
because the "lui" instruction is defined by the macro "Ip_u1s2(_lui)" -- I
believe it should be Ip_u1u2(_lui) -- and its definition is associated with
the SIMM macro -- I believe it should be the UIMM macro. The current code
takes a 32bit number and checks that it can be converted to a 16bit signed
immediate. This check fails of course for an immediate such as 0x0000a000.
This is fixed. However, there are two uses of uasm_i_lui() in
UASM_i_LA_mostly() which use 16bit signed immediates in the form of a
sign-extended 32 bit number. Left alone these may now cause a warning when
being processed by build_imm(). These two uses have been modified by first
calling build_simm() on the argument to uasm_i_lui() as to convert it to a
proper 16 bit unsigned integer.
Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
arch/mips/include/asm/uasm.h | 2 +-
arch/mips/mm/uasm-micromips.c | 2 +-
arch/mips/mm/uasm-mips.c | 2 +-
arch/mips/mm/uasm.c | 6 +++---
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/mips/include/asm/uasm.h b/arch/mips/include/asm/uasm.h
index f7effca791a5..7ea1d338570b 100644
--- a/arch/mips/include/asm/uasm.h
+++ b/arch/mips/include/asm/uasm.h
@@ -127,7 +127,7 @@ Ip_u2s3u1(_lh);
Ip_u2s3u1(_lhu);
Ip_u2s3u1(_ll);
Ip_u2s3u1(_lld);
-Ip_u1s2(_lui);
+Ip_u1u2(_lui);
Ip_u2s3u1(_lw);
Ip_u2s3u1(_lwu);
Ip_u3u1u2(_lwx);
diff --git a/arch/mips/mm/uasm-micromips.c b/arch/mips/mm/uasm-micromips.c
index 75ef90486fe6..86ee1499e120 100644
--- a/arch/mips/mm/uasm-micromips.c
+++ b/arch/mips/mm/uasm-micromips.c
@@ -82,7 +82,7 @@ static const struct insn insn_table_MM[insn_invalid] = {
[insn_lh] = {M(mm_lh32_op, 0, 0, 0, 0, 0), RT | RS | SIMM},
[insn_ll] = {M(mm_pool32c_op, 0, 0, (mm_ll_func << 1), 0, 0), RS | RT | SIMM},
[insn_lld] = {0, 0},
- [insn_lui] = {M(mm_pool32i_op, mm_lui_op, 0, 0, 0, 0), RS | SIMM},
+ [insn_lui] = {M(mm_pool32i_op, mm_lui_op, 0, 0, 0, 0), RS | UIMM},
[insn_lw] = {M(mm_lw32_op, 0, 0, 0, 0, 0), RT | RS | SIMM},
[insn_mfc0] = {M(mm_pool32a_op, 0, 0, 0, mm_mfc0_op, mm_pool32axf_op), RT | RS | RD},
[insn_mfhi] = {M(mm_pool32a_op, 0, 0, 0, mm_mfhi32_op, mm_pool32axf_op), RS},
diff --git a/arch/mips/mm/uasm-mips.c b/arch/mips/mm/uasm-mips.c
index 7154a1d99aad..b45c15111d68 100644
--- a/arch/mips/mm/uasm-mips.c
+++ b/arch/mips/mm/uasm-mips.c
@@ -132,7 +132,7 @@ static const struct insn insn_table[insn_invalid] = {
[insn_ll] = {M6(spec3_op, 0, 0, 0, ll6_op), RS | RT | SIMM9},
[insn_lld] = {M6(spec3_op, 0, 0, 0, lld6_op), RS | RT | SIMM9},
#endif
- [insn_lui] = {M(lui_op, 0, 0, 0, 0, 0), RT | SIMM},
+ [insn_lui] = {M(lui_op, 0, 0, 0, 0, 0), RT | UIMM},
[insn_lw] = {M(lw_op, 0, 0, 0, 0, 0), RS | RT | SIMM},
[insn_lwu] = {M(lwu_op, 0, 0, 0, 0, 0), RS | RT | SIMM},
[insn_lwx] = {M(spec3_op, 0, 0, 0, lwx_op, lx_op), RS | RT | RD},
diff --git a/arch/mips/mm/uasm.c b/arch/mips/mm/uasm.c
index c56f129c9a4b..ca5d47da3bd1 100644
--- a/arch/mips/mm/uasm.c
+++ b/arch/mips/mm/uasm.c
@@ -327,7 +327,7 @@ I_u2s3u1(_lh)
I_u2s3u1(_lhu)
I_u2s3u1(_ll)
I_u2s3u1(_lld)
-I_u1s2(_lui)
+I_u1u2(_lui)
I_u2s3u1(_lw)
I_u2s3u1(_lwu)
I_u1u2u3(_mfc0)
@@ -457,7 +457,7 @@ UASM_EXPORT_SYMBOL(uasm_rel_lo);
void UASM_i_LA_mostly(u32 **buf, unsigned int rs, long addr)
{
if (!uasm_in_compat_space_p(addr)) {
- uasm_i_lui(buf, rs, uasm_rel_highest(addr));
+ uasm_i_lui(buf, rs, build_simm(uasm_rel_highest(addr)));
if (uasm_rel_higher(addr))
uasm_i_daddiu(buf, rs, rs, uasm_rel_higher(addr));
if (uasm_rel_hi(addr)) {
@@ -468,7 +468,7 @@ void UASM_i_LA_mostly(u32 **buf, unsigned int rs, long addr)
} else
uasm_i_dsll32(buf, rs, rs, 0);
} else
- uasm_i_lui(buf, rs, uasm_rel_hi(addr));
+ uasm_i_lui(buf, rs, build_simm(uasm_rel_hi(addr)));
}
UASM_EXPORT_SYMBOL(UASM_i_LA_mostly);
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RESEND PATCH v1] MIPS: uasm: false warning on use of uasm_i_lui()
2020-09-08 16:45 [RESEND PATCH v1] MIPS: uasm: false warning on use of uasm_i_lui() Jim Quinlan
@ 2020-09-10 9:37 ` Thomas Bogendoerfer
2020-09-14 17:02 ` Jim Quinlan
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Bogendoerfer @ 2020-09-10 9:37 UTC (permalink / raw)
To: Jim Quinlan; +Cc: linux-mips, bcm-kernel-feedback-list, open list
On Tue, Sep 08, 2020 at 12:45:06PM -0400, Jim Quinlan wrote:
> Currently, the example uasm code
>
> uasm_i_lui(p, tmp, 0xa000);
>
> issues a warning at Linux boot when the code is "assembled". This is
> because the "lui" instruction is defined by the macro "Ip_u1s2(_lui)" -- I
> believe it should be Ip_u1u2(_lui) -- and its definition is associated with
> the SIMM macro -- I believe it should be the UIMM macro. The current code
> takes a 32bit number and checks that it can be converted to a 16bit signed
> immediate. This check fails of course for an immediate such as 0x0000a000.
IMHO SIMM is correct as the upper 16bits will be sign extended on 64bit
CPUs.
Your example looks like to try to load a KSEG1 address, so just use
uasm_i_lui(p, tmp, uasm_rel_hi(0xa0000000));
which even makes it clearer, what this is about.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RESEND PATCH v1] MIPS: uasm: false warning on use of uasm_i_lui()
2020-09-10 9:37 ` Thomas Bogendoerfer
@ 2020-09-14 17:02 ` Jim Quinlan
0 siblings, 0 replies; 3+ messages in thread
From: Jim Quinlan @ 2020-09-14 17:02 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: linux-mips, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, open list
On Thu, Sep 10, 2020 at 5:37 AM Thomas Bogendoerfer
<tsbogend@alpha.franken.de> wrote:
>
> On Tue, Sep 08, 2020 at 12:45:06PM -0400, Jim Quinlan wrote:
> > Currently, the example uasm code
> >
> > uasm_i_lui(p, tmp, 0xa000);
> >
> > issues a warning at Linux boot when the code is "assembled". This is
> > because the "lui" instruction is defined by the macro "Ip_u1s2(_lui)" -- I
> > believe it should be Ip_u1u2(_lui) -- and its definition is associated with
> > the SIMM macro -- I believe it should be the UIMM macro. The current code
> > takes a 32bit number and checks that it can be converted to a 16bit signed
> > immediate. This check fails of course for an immediate such as 0x0000a000.
>
> IMHO SIMM is correct as the upper 16bits will be sign extended on 64bit
> CPUs.
>
Hi Thomas,
Got it.
Thanks,
Jim
> Your example looks like to try to load a KSEG1 address, so just use
>
> uasm_i_lui(p, tmp, uasm_rel_hi(0xa0000000));
>
> which even makes it clearer, what this is about.
>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-14 17:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-08 16:45 [RESEND PATCH v1] MIPS: uasm: false warning on use of uasm_i_lui() Jim Quinlan
2020-09-10 9:37 ` Thomas Bogendoerfer
2020-09-14 17:02 ` Jim Quinlan
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).