From: Richard Sandiford <richard.sandiford@arm.com>
To: Richard Biener <rguenther@suse.de>
Cc: Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org>,
YunQiang Su <wzssyqa@gmail.com>,
Jeff Law <jeffreyalaw@gmail.com>,
Eric Botcazou <botcazou@adacore.com>,
YunQiang Su <yunqiang.su@cipunited.com>,
pinskia@gmail.com, ian@airs.com
Subject: Re: [PATCH v2] Store_bit_field_1: Use SUBREG instead of REG if possible
Date: Thu, 20 Jul 2023 10:22:30 +0100 [thread overview]
Message-ID: <mptjzuvj5t5.fsf@arm.com> (raw)
In-Reply-To: <nycvar.YFH.7.77.849.2307200722340.12935@jbgna.fhfr.qr> (Richard Biener's message of "Thu, 20 Jul 2023 07:23:37 +0000 (UTC)")
Richard Biener <rguenther@suse.de> writes:
#> 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 <botcazou@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?
Not easily. The problem is that the original 64-bit ISA says that the
behaviour of a 32-bit arithmetic instruction is undefined if the inputs
aren't in sign-extended form. (A bit like GCC CONST_INTs :)) So:
// $2 = 0x0_8000_0000
addiu $3,$2,$2
has undefined behaviour, it must instead be:
// $2 = 0xffff_ffff_8000_0000
The purpose of TRULY_NOOP_TRUNCATION is to ensure that we never form an
SImode register by taking a lowpart subreg of a DImode (or wider) register.
In other words, things are inverted to that truncating DI to SI is a
sign-extend operation (represented in RTL as a trunc) while sign-extending
from SI to DI is free (lowered to nothing after RA).
Richard
next prev parent reply other threads:[~2023-07-20 9:22 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-19 4:16 YunQiang Su
2023-07-19 6:26 ` Richard Biener
2023-07-19 6:58 ` YunQiang Su
2023-07-19 7:22 ` Richard Biener
2023-07-19 8:21 ` YunQiang Su
2023-07-19 8:25 ` YunQiang Su
2023-07-19 8:50 ` YunQiang Su
2023-07-19 9:23 ` Richard Biener
2023-07-19 9:27 ` Richard Biener
2023-07-19 9:43 ` YunQiang Su
2023-07-19 9:45 ` Eric Botcazou
2023-07-19 10:12 ` YunQiang Su
2023-07-19 10:25 ` Richard Biener
2023-07-19 12:22 ` Jeff Law
2023-07-20 7:09 ` Richard Sandiford
2023-07-20 7:23 ` Richard Biener
2023-07-20 9:22 ` Richard Sandiford [this message]
2023-12-22 6:11 ` YunQiang Su
2023-12-22 4:45 ` YunQiang Su
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=mptjzuvj5t5.fsf@arm.com \
--to=richard.sandiford@arm.com \
--cc=botcazou@adacore.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ian@airs.com \
--cc=jeffreyalaw@gmail.com \
--cc=pinskia@gmail.com \
--cc=rguenther@suse.de \
--cc=wzssyqa@gmail.com \
--cc=yunqiang.su@cipunited.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 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 read-only IMAP folder(s) and NNTP newsgroup(s).