public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <jeffreyalaw@gmail.com>
To: Vineet Gupta <vineetg@rivosinc.com>, gcc-patches@gcc.gnu.org
Cc: kito.cheng@gmail.com, Palmer Dabbelt <palmer@rivosinc.com>,
	gnu-toolchain@rivosinc.com
Subject: Re: [PATCH] RISC-V: zicond: remove bogus opt2 pattern
Date: Fri, 1 Sep 2023 07:13:00 -0600	[thread overview]
Message-ID: <e157aff9-94a9-40d6-f7e7-6a747b50b0c3@gmail.com> (raw)
In-Reply-To: <d24bb347-9677-46f6-984a-1af704ea9208@rivosinc.com>



On 8/31/23 11:57, Vineet Gupta wrote:
> 
> 
> On 8/31/23 06:51, Jeff Law wrote:
>>
>>
>> On 8/30/23 15:57, Vineet Gupta wrote:
>>> This was tripping up gcc.c-torture/execute/pr60003.c at -O1 since the
>>> pattern semantics can't be expressed by zicond instructions.
>>>
>>> This involves test code snippet:
>>>
>>>        if (a == 0)
>>>     return 0;
>>>        else
>>>     return x;
>>>      }
>>>
>>> which is equivalent to:  "x = (a != 0) ? x : a"
>> Isn't it
>>
>> x = (a == 0) ? 0 : x
>>
>> Which seems like it ought to fit zicond just fine.
> 
> Logically they are equivalent, but ....
> 
>>
>> If we take yours;
>>
>> x = (a != 0) ? x : a
>>
>> And simplify with the known value of a on the false arm we get:
>>
>> x = (a != 0 ) ? x : 0;
>>
>> Which is equivalent to
>>
>> x = (a == 0) ? 0 : x;
>>
>> So ISTM this does fit zicond just fine.
> 
> I could very well be mistaken, but define_insn is a pattern match and 
> opt2 has *ne* so the expression has to be in != form and thus needs to 
> work with that condition. No ?
My point was  that

x = (a != 0) ? x : 0

is equivalent to

x = (a == 0) ? 0 : x

You can invert the condition and swap the arms and get the same 
semantics.  Thus if one can be supported, so can the other as they're 
functionally equivalent.  It may be the at we've goof'd something in 
handling the inverted case, but conceptually we ought to be able to 
handle both.

I don't doubt you've got a failure, but it's also the case that I'm not 
seeing the same failure when I turn on zicond and run the execute.exp 
tests.  So clearly there's a difference somewhere in what we're doing.

So perhaps we should start with comparing assembly output for the test 
in question.  Can you pass yours along, I'll diff them this afternoon 
and see what we find.

jeff

  reply	other threads:[~2023-09-01 13:13 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-30 21:57 Vineet Gupta
2023-08-31 13:51 ` Jeff Law
2023-08-31 17:57   ` Vineet Gupta
2023-09-01 13:13     ` Jeff Law [this message]
2023-09-01 18:55       ` Vineet Gupta
2023-09-01 17:40     ` Palmer Dabbelt
2023-09-01 19:17       ` Vineet Gupta

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=e157aff9-94a9-40d6-f7e7-6a747b50b0c3@gmail.com \
    --to=jeffreyalaw@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=gnu-toolchain@rivosinc.com \
    --cc=kito.cheng@gmail.com \
    --cc=palmer@rivosinc.com \
    --cc=vineetg@rivosinc.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).