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.
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
