On 12/6/24 6:12 AM, Oliver Kozul wrote:
The patch optimizes code generation for comparisons of the form
X & C1 == C2 by converting them to (X | ~C1) == (C2 | ~C1).
C1 is a constant that requires li and addi to be loaded,
while ~C1 requires a single lui instruction.
2024-12-06 Oliver Kozul <oliver.ko...@rt-rk.com>
PR target/114087
gcc/ChangeLog:
* config/riscv/riscv.md (*lui_constraint<ANYI:mode>_and_to_or):
New pattern.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/pr114087-1.c: New test.
A few comments:
+(define_insn_and_split "*lui_constraint<ANYI:mode>_and_to_or"
+ [(set (match_operand:ANYI 0 "register_operand" "=r")
+ (plus:ANYI (and:ANYI (match_operand:ANYI 1 "register_operand" "r")
+ (match_operand 2 "const_int_operand"))
+ (match_operand 3 "const_int_operand")))
+ (clobber (match_scratch:X 4 "=&r"))]
Similar formatting comment as I had on the other patch. Easily fixed.
Just for giggles I asked chatgpt to format it and it came up with:
(define_insn_and_split "*lui_constraint<ANYI:mode>_and_to_or"
[(set (match_operand:ANYI 0 "register_operand" "=r")
(plus:ANYI (and:ANYI (match_operand:ANYI 1 "register_operand" "r")
(match_operand 2 "const_int_operand"))
(match_operand 3 "const_int_operand")))
(clobber (match_scratch:X 4 "=&r"))]
"LUI_OPERAND (INTVAL (operands[2]) + 1)
&& (INTVAL (operands[2]) & (-INTVAL (operands[3]))) == (-INTVAL
(operands[3]))"
"#"
"&& reload_completed"
[(set (match_dup 4) (match_dup 5))
(set (match_dup 0) (ior:X (match_dup 1) (match_dup 4)))
(set (match_dup 4) (match_dup 6))
(set (match_dup 0) (minus:X (match_dup 0) (match_dup 4)))]
{
operands[5] = GEN_INT (~INTVAL (operands[2]));
operands[6] = GEN_INT ((~INTVAL (operands[2])) | (-INTVAL (operands[3])));
}
[(set_attr "type" "arith")])
Which looks basically correct.
More importantly we probably want a comment on what this pattern is
actually doing. Perhaps something like:
;; Transform (X & C1) + C2 into (X | ~C1) - (-C2 | ~C1)
;; Where C1 is not a LUI operand, but ~C1 is a LUI operand
Did I get that correct?
+ "LUI_OPERAND (INTVAL (operands[2]) + 1)
So shouldn't this have been LUI_OPERAND (~INTVAL (operands[2]))? I can
kind-of see why you used +1, but given the C fragment in the pattern
uses ~operands[2], let's use the same here for the test if we can.
+ && (INTVAL (operands[2]) & (-INTVAL (operands[3])))
+ == (-INTVAL (operands[3]))"
Does it make sense to verify that operands[3] is a constant that
requires synthesis and that ~INTVAL (operands[2]) | -INTVAL
(operands[3]) is no more expensive to synthesize than operands[3]?
ie, saving the addi on the first constant isn't a win if the second
constant gets more complex to synthesize. The API isn't great, but you
can use riscv_const_insns to find out how many instructions are
necessary to synthesize a constant.
Overall it looks good. Just needs a bit of polishing.
jeff
