Re: [PATCH] Add new target hook: simplify_modecc_const.

["H.J. Lu" <hjl.tools@gmail.com>]

On Fri, Oct 14, 2022 at 1:32 PM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
> On 10/10/22 09:50, H.J. Lu via Gcc-patches wrote:
> > On Thu, Jul 28, 2022 at 5:40 AM Richard Sandiford via Gcc-patches
> > <gcc-patches@gcc.gnu.org> 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" <roger@nextmovesoftware.com> 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])))
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107172
> >
> > This new target hook handles it properly
>
> ANd does it work if you reject MODE_CC comparisons with two constants in
> simplify_const_relational_operation?
>
>
> I suspect it will work, but generate suboptimal code.

It doesn't work for

(ltu:SI (const_int 1 [0x1]) (const_int 0 [0]))

simplified from

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

When simplify_const_relational_operation returns NULL for
MODE_CC comparison with two constants, combine will try
it again with VOIDmode comparison with two constants.

-- 
H.J.