public inbox for
 help / color / mirror / Atom feed
From: "H.J. Lu" <>
To: Richard Sandiford <>,
	Roger Sayle <>,
	 Segher Boessenkool <>,
Subject: Re: [PATCH] Add new target hook: simplify_modecc_const.
Date: Mon, 10 Oct 2022 08:50:33 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Thu, Jul 28, 2022 at 5:40 AM Richard Sandiford via Gcc-patches
<> wrote:
> Seems this thread has become a bit heated, so I'll try to proceed
> with caution :-)
> In the below, I'll use "X-mode const_int" to mean "a const_int that
> is known from context to represent an X-mode value".  Of course,
> the const_int itself always stores VOIDmode.
> "Roger Sayle" <> writes:
> > Hi Segher,
> > It's very important to distinguish the invariants that exist for the RTL
> > data structures as held in memory (rtx), vs. the use of "enum rtx_code"s,
> > "machine_mode"s and operands in the various processing functions
> > of the middle-end.
> FWIW, I agree this distinction is important, with the proviso (which
> I think you were also adding) that the code never loses track of what
> mode an rtx operand (stored in a variable) actually has/is being
> interpreted to have.
> In other words, the reason (zero_extend (const_int N)) is invalid is
> not that constant integers can't be extended in principle (of course
> they can).  It's invalid because we've lost track of how many bits
> that N actually has.  That problem doesn't apply in contexts where
> the operation is described using individual variables (rather than
> a single rtx) *provided that* one of those variables says what mode
> any potential const_ints actually represent.
> > Yes, it's very true that RTL integer constants don't specify a mode
> > (are VOIDmode), so therefore operations like ZERO_EXTEND or EQ
> > don't make sense with all constant operands.  This is (one reason)
> > why constant-only operands are disallowed from RTL (data structures),
> > and why in APIs that perform/simplify these operations, the original
> > operand mode (of the const_int(s)) must be/is always passed as a
> > parameter.
> >
> > Hence, for say simplify_const_binary_operation, op0 and op1 can
> > both be const_int, as the mode argument specifies the mode of the
> > "code" operation. Likewise, in simplify_relational_operation, both
> > op0 and op1 may be CONST_INT as "cmp_mode" explicitly specifies
> > the mode that the operation is performed in and "mode" specifies
> > the mode of the result.
> And the mode argument to simplify_const_relational_operation specifies
> the mode of the operands, not the mode of the result.  I.e. it specifies
> the modes of op0 and op1 rather than the mode that would be attached to
> the code in "(code:mode ...)" if an rtx were created with these parameters.
> That confused me when I saw the patch initially.  Elsewhere in the
> file "mode" tends to be the mode of the result, in cases where the
> mode of the result can be different from the modes of the operands,
> so using it for the mode of the operands seems a bit confusing
> (not your fault of course).
> I still struggle with the idea of having CC-mode const_ints though
> (using the meaning of "CC-mode const_ints" above).  I realise
> (compare (...) (const_int 0)) has been the norm "for ever", but here
> it feels like we're also blessing non-zero CC-mode const_ints.
> That raises the question of how many significant bits a CC-mode
> const_int actually has.  Currently:
>          ...  For historical reasons,
>          the size of a CC mode is four units.
> But treating CC-mode const_ints as having 32 significant bits is surely
> the wrong thing to do.
> So if we want to add more interpretation around CC modes, I think we
> should first clean up the representation to make the set of valid values
> more explicit.  (Preferably without reusing const_int for constant values,
> but that's probably a losing battle :-))
> Thanks,
> Richard

Here is a testcase to show that combine generates

(set (reg:CCC 17 flags)
       (ltu:SI (const_int 1 [1])
         (const_int 0 [0])))

This new target hook handles it properly.


  reply	other threads:[~2022-10-10 15:51 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-07 21:08 [PATCH] Be careful with MODE_CC in simplify_const_relational_operation Roger Sayle
2022-07-07 22:38 ` Segher Boessenkool
2022-07-26 12:13   ` [PATCH] Add new target hook: simplify_modecc_const Roger Sayle
2022-07-26 17:44     ` Segher Boessenkool
2022-07-26 21:04       ` Roger Sayle
2022-07-26 22:11         ` Segher Boessenkool
2022-07-27  7:51           ` Roger Sayle
2022-07-27 18:37             ` Segher Boessenkool
2022-07-28 12:39         ` Richard Sandiford
2022-10-10 15:50           ` H.J. Lu [this message]
2022-10-14 20:31             ` Jeff Law
2022-10-14 21:05               ` H.J. Lu
2022-10-14 20:26       ` 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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \

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