On Thu, 20 Jul 2023, Richard Sandiford wrote:
> Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > On 7/19/23 04:25, Richard Biener wrote:
> >> On Wed, 19 Jul 2023, YunQiang Su wrote:
> >>
> >>> Eric Botcazou <botca...@adacore.com> ?2023?7?19??? 17:45???
> >>>>
> >>>>> I don't see that. That's definitely not what GCC expects here,
> >>>>> the left-most word of the doubleword should be unchanged.
> >>>>>
> >>>>> Your testcase should be a dg-do-run and probably more like
> >>>>>
> >>>>> NOMIPS16 int __attribute__((noipa)) test (const unsigned char *buf)
> >>>>> {
> >>>>> int val;
> >>>>> ((unsigned char*)&val)[0] = *buf++;
> >>>>> ((unsigned char*)&val)[1] = *buf++;
> >>>>> ((unsigned char*)&val)[2] = *buf++;
> >>>>> ((unsigned char*)&val)[3] = *buf++;
> >>>>> return val;
> >>>>> }
> >>>>> int main()
> >>>>> {
> >>>>> int val = 0x01020304;
> >>>>> val = test (&val);
> >>>>> if (val != 0x01020304)
> >>>>> abort ();
> >>>>> }
> >>>>>
> >>>>> not sure if I got endianess correct. Now, the question is what
> >>>>> WORD_REGISTER_OPERATIONS implies for a bitfield insert and what
> >>>>> the MIPS ABI says for returning SImode.
> >>>>
> >>>
> >>> MIPS N64 ABI uses 2 GPR for integer return values.
> >>> If the return value is SImode, the first v0 register is used, and it
> >>> must be sign-extended,
> >>> aka the bits[64-31] are all same.
> >>>
> >>> Yes, it is same for signed and unsigned int32.
> >>>
> >>> https://irix7.com/techpubs/007-2816-004.pdf
> >>> Page 6:
> >>> 32-bit integer (int) parameters are always sign-extended when passed
> >>> in registers,
> >>> whether of signed or unsigned type. [This issue does not arise in the
> >>> o32-bit ABI.]
> >>
> >> Note I think Andrews comment#7 in the PR is spot-on then, the issue
> >> isn't the bitfield inserts but the compare where combine elides
> >> the sign_extend in favor of a subreg. That's likely some wrongdoing
> >> in simplify-rtx in the context of WORD_REGISTER_OPERATIONS.
> > And I think it raises a real question about the use of GPR (which maps
> > to SImode and DImode for 64bit MIPS targets) on the conditional
> > branching patterns in mips.md.
> >
> > So while this code works:
> >
> >> (insn 20 19 23 2 (set (reg/v:DI 200 [ val+-4 ])
> >> (sign_extend:DI (subreg:SI (reg/v:DI 200 [ val+-4 ]) 4)))
> >> "/app/example.cpp":7:29 -1
> >> (nil))
>
> Haven't had chance to compile and look at it properly, but this subreg
> seems suspicious for MIPS, given the definition of TRULY_NOOP_TRUNCATION.
> We should instead use a truncdisi2 to narrow reg:DI 200 to an SI register,
> and then sign_extend it.
>
> This is easily missed in target-independent code because so few targets
> define TRULY_NOOP_TRUNCATION.
Can we easily get rid of it?
Richard.
> Where is the subreg being generated?
>
> Richard
>
> >> (jump_insn 23 20 24 2 (set (pc)
> >> (if_then_else (le (subreg/s/u:SI (reg/v:DI 200 [ val+-4 ]) 4)
> >> (const_int 0 [0]))
> >> (label_ref 32)
> >> (pc))) "/app/example.cpp":8:5 -1
> >> (int_list:REG_BR_PROB 440234148 (nil))
> >> -> 32)
> >
> >
> > Normally the narrowing SUBREG in insn 23 would indicate we don't care
> > about the bits outside SImode. But on a W_R_O targets we very much care
> > because the hardware is going to ultimately do the comparison in 64 bits.
> >
> > As Andrew/Richi have indicated this very much points to combine as
> > incorrectly eliminating the explict sign extension. Most likely because
> > something saw the SUBREG and concluded those upper bits set by insn 20
> > were "don't care" bits.
> >
> > But it may ultimately be be better for the MIPS port to not expose a
> > SImode comparison. Thus reducing the reliance on W_R_O and its
> > under-specified semantics and ultimately having the RTL map more closely
> > to what the hardware actually does/supports.
> >
> > That's the model we're working towards on the RISC-V port as well. I
> > wouldn't be surprised if we eventually get to the point where we
> > eliminate WORD_REGISTER_OPERATIONS entirely.
> >
> > And yes, bitfield operations are one of the nasty sticking points. The
> > thinking for them is that we want to support bit manipulations where the
> > bit position is variable. To do that we will emit an explicit sign
> > extension after such operations. Then rely on improved REE to identify
> > and remove those redundant extensions.
> >
> > Jeff
> >
> > Jeff
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
