On 5/13/24 12:49 PM, Vineet Gupta wrote:
Apologies for the delay in getting this out. Needed to fix one ICE
with glibc build and fresh round of testing: both testsuite and SPEC
runs (which are similar to v1 in terms of Cactu gains, but some more minor
regressions elsewhere gcc). Again those seem so small that IMHO this
should still go in.
I'll investigate those next as well as an existing weirdnes in glibc tempnam
which I spotted during the debugging.
Changes since v1 [1]
- Tighten the main conditition to avoid stack regs as destination
(to avoid making them potentially unaligned with -2047 addend:
this might be OK execution/ABI wise, but undesirable/ugly still
specially when coming from compiler codegen).
- Ensure that first alternative is always split
- Remove "&& 1" from split condition. That was tripping up glibc build
with illegal operands `add s0, s0, 2048`.
[1] https://gcc.gnu.org/pipermail/gcc-patches/2024-March/647877.html
+;; Special case of adding a reg and constant if latter is sum of two S12
+;; values (in range -2048 to 2047). Avoid materialized the const and fuse
+;; into the add (with an additional add for 2nd value). Makes a 3 insn
+;; sequence into 2 insn.
+
+(define_insn_and_split "*add<mode>3_const_sum_of_two_s12"
+ [(set (match_operand:P 0 "register_operand" "=r,r")
+ (plus:P (match_operand:P 1 "register_operand" " r,r")
+ (match_operand:P 2 "const_two_s12" " MiG,r")))]
+ "!riscv_reg_frame_related (operands[0])"
So that !riscv_reg_frame_related is my only concern with this patch.
It's a destination, so it *may* be OK.
If it were a source operand, then we'd have to worry about cases where
it was a pseudo with the same value as sp/fp/argp and subsequent copy
propagation replacing the pseudo with sp/fp/argp causing the insn to no
longer match.
Similarly if it were a source operand we'd have to worry about cases
where the pseudo had a registered (or discoverable) equivalence to
sp/fp/argp plus an offset. IRA/LRA can replace the use with its
equivalence in some of those cases which would have potentially caused
headaches.
But as a destination we really just have to worry about generation in
the prologue/epilogue and for alloca calls. Those should be the only
places that set one of those special registers. They're constrained
enough that I think we'll be OK.
I'm very slightly worried about hard register cprop, but I think it
should be safe these days WRT those special registers in the unlikely
event it found an opportunity to propagate them.
So a tentative OK. If we find this tidibit is problematical in the
future, then what I would suggest is we allow those special registers
and dial-back the aggressiveness on the range of allowed constants.
That would allow the first instruction in the sequence to never create a
mis-aligned sp. But again, that's only if we need to revisit.
Please wait for CI to report back sane results :-)
Jeff
