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" <ro...@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.
