public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <jeffreyalaw@gmail.com>
To: Jakub Jelinek <jakub@redhat.com>,
	Richard Biener <rguenther@suse.de>,
	Eric Botcazou <botcazou@adacore.com>,
	gcc-patches@gcc.gnu.org, richard.sandiford@arm.com
Subject: Re: [PATCH] dse: Handle SUBREGs of word REGs differently for WORD_REGISTER_OPERATIONS targets [PR109040]
Date: Thu, 6 Apr 2023 08:45:59 -0600	[thread overview]
Message-ID: <21e6eec0-a49f-954a-5f3e-5a998163bb6b@gmail.com> (raw)
In-Reply-To: <mptlej5mk6b.fsf@arm.com>



On 4/6/23 03:31, Richard Sandiford wrote:
> Jeff Law <jeffreyalaw@gmail.com> writes:
>> On 4/5/23 10:48, Jakub Jelinek wrote:
>>> On Wed, Apr 05, 2023 at 10:17:59AM -0600, Jeff Law wrote:
>>>>> It is true that an instruction like
>>>>> (insn 8 7 9 2 (set (reg:HI 141)
>>>>>            (subreg:HI (reg:SI 142) 0)) "aauu.c":6:18 181 {*movhi_internal}
>>>>>         (nil))
>>>>> can appear in the IL on WORD_REGISTER_OPERATIONS target, but I think the
>>>>> upper bits shouldn't be random garbage in that case, it should be zero
>>>>> extended or sign extended.
>>>> Well, that's one of the core questions here.  What are the state of the
>>>> upper 16 bits of (reg:HI 141)?  The WORD_REGISTER_OPERATIONS docs aren't
>>>> 100% clear as we're not really doing any operation.
>>>>
>>>> So again, I think we need to decide if the DSE transformation is correct or
>>>> not.  I *think* we can aggree that insn 39 is OK.  It's really the semantics
>>>> of insn 47 that I think we need to agree on.  What is the state of the upper
>>>> 16 bits of (reg:HI 175) after insn 47?
>>>
>>> I'm afraid I don't know the answers here, I think Eric is
>>> WORD_REGISTER_OPERATIONS expert here I think these days (most of the major
>>> targets are !WORD_REGISTER_OPERATIONS).
>> Hopefully he'll chime in.
> 
> Just curious: have you experimented with making RISC-V
> !WORD_REGISTER_OPERATIONS too?  Realise it's not the right way
> to fix the bug, just curious in general.
We haven't experimented with it AFAIK.  We don't have a full set of 
SImode operations, but we may have enough of a subset to try. 
Alternately, we could potentially see what happens if we ignore the 
32bit ops that we do support.  Both general directions are probably 
worth exploring, but not right now and probably not even for gcc-14 
(where we're going to be busy as hell on the vector side).

> 
> Not defining it seems to have worked well for AArch64.  And IMO
> the semantics are much easier to follow when there is no special
> treatment of upper bits.  Subregs are hard enough to reason about
> as it is...
Amen to that.  My sense is that the risc-v port relies far too heavily 
on SUBREGs and the WORD_REGISTER_OPERATIONS just makes reasoning about 
correctness harder.

jeff

  parent reply	other threads:[~2023-04-06 14:46 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-05  9:16 Jakub Jelinek
2023-04-05 13:14 ` Jeff Law
2023-04-05 14:51   ` Jakub Jelinek
2023-04-05 16:17     ` Jeff Law
2023-04-05 16:48       ` Jakub Jelinek
2023-04-05 17:31         ` Jeff Law
2023-04-06  9:31           ` Richard Sandiford
2023-04-06  9:37             ` Li, Pan2
2023-04-06 14:49               ` Jeff Law
2023-04-06 14:45             ` Jeff Law [this message]
2023-04-06 10:15           ` Eric Botcazou
2023-04-06 10:31             ` [PATCH] combine: Fix simplify_comparison AND handling " Jakub Jelinek
2023-04-06 10:51               ` Eric Botcazou
2023-04-06 11:37                 ` Jakub Jelinek
2023-04-06 14:21                   ` Eric Botcazou
2023-04-09  0:25                     ` Jeff Law
2023-04-10  7:10                       ` Jakub Jelinek
2023-04-12  1:26                         ` Jeff Law
2023-04-12  6:21                           ` Jakub Jelinek
2023-04-12 10:02                             ` [PATCH] combine, v3: Fix " Jakub Jelinek
2023-04-12 14:17                               ` Jeff Law
2023-04-12 14:30                                 ` Jakub Jelinek
2023-04-12 15:24                               ` Segher Boessenkool
2023-04-12 16:58                               ` [PATCH] combine, v4: " Jakub Jelinek
2023-04-13  4:05                                 ` Jeff Law
2023-04-13 10:57                                   ` Segher Boessenkool
2023-04-13 12:35                                     ` Jeff Law
2023-04-13 13:45                                       ` [PATCH] loop-iv: Fix up bounds computation Jakub Jelinek
2023-04-13 15:07                                         ` Jeff Law
2023-04-13 19:37                                         ` Jeff Law
2023-04-12 13:29                             ` [PATCH] combine: Fix simplify_comparison AND handling for WORD_REGISTER_OPERATIONS targets [PR109040] Jeff Law
2023-04-09  1:15                   ` Jeff Law
2023-04-10  5:13                     ` Hongtao Liu
2023-04-10  5:15                       ` Hongtao Liu
2023-04-06 14:35               ` Jeff Law
2023-04-06 15:06               ` Jeff Law
2023-04-06 14:53             ` [PATCH] dse: Handle SUBREGs of word REGs differently " Jeff Law

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=21e6eec0-a49f-954a-5f3e-5a998163bb6b@gmail.com \
    --to=jeffreyalaw@gmail.com \
    --cc=botcazou@adacore.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=rguenther@suse.de \
    --cc=richard.sandiford@arm.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).