On Mon, Feb 10, 2025 at 12:26:03AM +0100, Uros Bizjak wrote:
> On Sun, Feb 9, 2025 at 11:31 PM Jakub Jelinek <[email protected]> wrote:
> >
> > On Sun, Feb 09, 2025 at 09:24:30AM +0100, Uros Bizjak wrote:
> > > "For commutative and comparison operators, a constant is always made
> > > the second operand."
> >
> > Isn't that just for commutative comparison operators (eq, ne, ordered,
> > unordered, ltgt, uneq)? Compare itself even isn't RTX_COMPARE at all,
> > it is RTX_BIN_ARITH, so similar to minus.
> > And I must say I can't find where simplify-rtx.cc or combine.cc would be
> > trying to canonicalize that order (note, it would need to change the related
> > uses of flags as well).
>
> When combine combines some RTX with any operand of COMPARE RTX, it
> canonicalizes COMPARE RTX in simplify_compare_const:
>
> /* Try to simplify a comparison between OP0 and a constant OP1,
> where CODE is the comparison code that will be tested, into a
> (CODE OP0 const0_rtx) form.
>
> The result is a possibly different comparison code to use.
> *POP0 and *POP1 may be updated. */
>
> This form won't match the new BT RTX.
>
> At the moment, there is no instruction using compare:CCC that could be
> combined with e.g. ZERO_EXTRACT RTX to form "*bt<mode>" insn, so the
> canonicalization rules do not matter, because BT is always expanded
> from splitters in a kind-of manual way. This is the reason that patch
> is OK, but without adding the new form to
> ix86_canonicalize_comparison, combine won't be able to match the new
> BT RTX in future.
>
> Please see ix86_canonicalize_comparison for a couple of examples on
> how to allow combine to match non-canonical compares.
So, would it be better to represent *bt<mode> as
(set (reg:CCC FLAGS_REG)
(compare:CCC (not:SWI48 (zero_extract:SWI48 ...)) (const_int -1)))
rather than
(set (reg:CCC FLAGS_REG)
(compare:CCC (const_int 0) (zero_extract:SWI48 ...)))
?
Or
(set (reg:CCC FLAGS_REG)
(compare:CCC (xor:SWI48 (zero_extract:SWI48 ...) (const_int 1)) (const_int
1)))
Though, simplify_compare_const has similarly code to transform
(ltu something (const_int C)) into (leu something (const_int C-1))
and that would affect both.
Or do you want additionally:
--- gcc/config/i386/i386.cc 2025-02-07 14:25:03.982866595 +0100
+++ gcc/config/i386/i386.cc 2025-02-10 09:39:36.914272948 +0100
@@ -595,6 +595,19 @@ ix86_canonicalize_comparison (int *code,
*code = (int) swap_condition ((enum rtx_code) *code);
return;
}
+
+ /* BT is represented as
+ (compare:CCC (const_int 0) (zero_extract ... (const_int 1) ...)) */
+ if (!op0_preserve_value
+ && (*code == GTU || *code == LEU)
+ && GET_CODE (*op0) == ZERO_EXTRACT
+ && XEXP (*op0, 1) == const1_rtx
+ && *op1 == const0_rtx)
+ {
+ std::swap (*op0, *op1);
+ *code = (int) swap_condition ((enum rtx_code) *code);
+ return;
+ }
}
/* Hook to determine if one function can safely inline another. */
or so? But what will care to use CCCmode for this rather than some other?
Jakub
