public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <jeffreyalaw@gmail.com>
To: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] riscv: implement TARGET_MODE_REP_EXTENDED
Date: Fri, 16 Sep 2022 17:48:24 -0600	[thread overview]
Message-ID: <4d4edef0-8a44-d6b8-0f50-1abd7f5e91f4@gmail.com> (raw)
In-Reply-To: <ba895f78-7f47-0f4-5bfe-e21893c4bcb@ispras.ru>


On 9/6/22 05:39, Alexander Monakov via Gcc-patches wrote:
> On Mon, 5 Sep 2022, Philipp Tomsich wrote:
>
>> +riscv_mode_rep_extended (scalar_int_mode mode, scalar_int_mode mode_rep)
>> +{
>> +  /* On 64-bit targets, SImode register values are sign-extended to DImode.  */
>> +  if (TARGET_64BIT && mode == SImode && mode_rep == DImode)
>> +    return SIGN_EXTEND;
> I think this leads to a counter-intuitive requirement that a hand-written
> inline asm must sign-extend its output operands that are bound to either
> signed or unsigned 32-bit lvalues. Will compiler users be aware of that?

Is this significantly different than on MIPS?  Hand-written code there 
also has to ensure that the results are properly sign extended and it's 
been that way for 20+ years since the introduction of mips64 IIRC.  
Though I don't think we had MODE_REP_EXTENDED that long.

Haha, MIPS is the only target that currently defines 
TARGET_MODE_REP_EXTENDED :-)



>
> Moreover, without adjusting TARGET_TRULY_NOOP_TRUNCATION this should cause
> miscompilation when a 64-bit variable is truncated to 32 bits: the pre-existing
> hook says that nothing needs to be done to truncate, but the new hook says
> that the result of the truncation is properly sign-extended.
>
> The documentation for TARGET_MODE_REP_EXTENDED warns about that:
>
>      In order to enforce the representation of mode, TARGET_TRULY_NOOP_TRUNCATION
>      should return false when truncating to mode.

This may well need adjusting in Philipp's patch.   I'd be surprised if 
the MIPS definition wasn't usable nearly verbatim here.


jeff


  reply	other threads:[~2022-09-16 23:48 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-05 21:44 Philipp Tomsich
2022-09-06 11:39 ` Alexander Monakov
2022-09-16 23:48   ` Jeff Law [this message]
2022-09-17  7:59     ` Palmer Dabbelt
2022-11-04 23:00   ` Philipp Tomsich
2022-11-05  6:16     ` [PATCH] [PHIOPT] Add A ? B + CST : B match and simplify optimizations Zhongyunde
2022-11-05  6:34       ` Andrew Pinski
2022-11-05  9:03         ` Zhongyunde
2022-11-08 14:58           ` Richard Biener
2022-11-08 15:51             ` 钟云德
2022-11-09  8:00               ` Richard Biener
2022-11-07 13:55     ` [PATCH] riscv: implement TARGET_MODE_REP_EXTENDED Alexander Monakov
2022-11-08 23:45       ` Philipp Tomsich
2022-11-09 17:21         ` Alexander Monakov
2022-11-20 16:09     ` Jeff Law
2022-11-21 13:49       ` Alexander Monakov
2022-11-21 14:56         ` Jeff Law
2022-11-21 15:33           ` Alexander Monakov

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=4d4edef0-8a44-d6b8-0f50-1abd7f5e91f4@gmail.com \
    --to=jeffreyalaw@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    /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).