Noticed that new sum of two s12 splitter was generating following: | 059930 <tempnam>: | 59930: add sp,sp,-16 | 59934: lui t0,0xfffff | 59938: sd s0,0(sp) | 5993c: sd ra,8(sp) | 59940: add sp,sp,t0 | 59944: add s0,sp,2047 <---- | 59948: mv a2,a0 | 5994c: mv a3,a1 | 59950: mv a0,sp | 59954: li a4,1 | 59958: lui a1,0x1 | 5995c: add s0,s0,1 <--- | 59960: jal 59a3c
SP here becomes unaligned, even if transitively which is undesirable as well as incorrect: - ABI requires stack to be 8 byte aligned - asm code looks weird and unexpected - to the user it might falsely seem like a compiler bug even when not, specially when staring at asm for debugging unrelated issue. Fix this by using 2032+addend idiom when handling register operands related to stack. This only affects positive S12 values, negative values are already -2048 based. Unfortunately implementation requires making a copy of splitter, since it needs different varaints of predicate and constraint which cant be done conditionally in the same MD pattern (constraint with restricted range prevents LRA from allowing such insn despite new predicate) With the patch, we get following correct code instead: | .. | 59944: add s0,sp,2032 | .. | 5995c: add s0,s0,16 gcc/Changelog: * config/riscv/riscv.h: Add alignment arg to new macros. * config/riscv/constraints.md: Variant of new constraint. * config/riscv/predicates.md: Variant of new predicate. * config/riscv/riscv.md: Variant of new splitter which offsets off of 2032 (vs. 2047). Gate existing splitter behind riscv_reg_frame_related. * config/riscv/riscv.cc (riscv_reg_frame_related): New helper to conditionalize the existing and new spitters. * config/riscv/riscv-protos.h: Add new prototype. Signed-off-by: Vineet Gupta <vine...@rivosinc.com> --- gcc/config/riscv/constraints.md | 6 ++++++ gcc/config/riscv/predicates.md | 8 ++++++- gcc/config/riscv/riscv-protos.h | 1 + gcc/config/riscv/riscv.cc | 11 ++++++++++ gcc/config/riscv/riscv.h | 15 ++++++++----- gcc/config/riscv/riscv.md | 37 ++++++++++++++++++++++++++++++--- 6 files changed, 69 insertions(+), 9 deletions(-) diff --git a/gcc/config/riscv/constraints.md b/gcc/config/riscv/constraints.md index 435461180c7e..a9446c54ee45 100644 --- a/gcc/config/riscv/constraints.md +++ b/gcc/config/riscv/constraints.md @@ -86,6 +86,12 @@ (ior (match_test "IN_RANGE (ival, 2048, 4094)") (match_test "IN_RANGE (ival, -4096, -2049)")))) +(define_constraint "MiA" + "const can be represented as sum of two S12 values with first aligned." + (and (match_code "const_int") + (ior (match_test "IN_RANGE (ival, 2033, 4064)") + (match_test "IN_RANGE (ival, -4096, -2049)")))) + (define_constraint "Ds3" "@internal 1, 2 or 3 immediate" diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md index 89490339c2da..56f9919daafa 100644 --- a/gcc/config/riscv/predicates.md +++ b/gcc/config/riscv/predicates.md @@ -423,7 +423,13 @@ (define_predicate "const_two_s12" (match_code "const_int") { - return SUM_OF_TWO_S12 (INTVAL (op)); + return SUM_OF_TWO_S12 (INTVAL (op), false); +}) + +(define_predicate "const_two_s12_algn" + (match_code "const_int") +{ + return SUM_OF_TWO_S12 (INTVAL (op), true); }) ;; CORE-V Predicates: diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h index b87355938052..f9e407bf5768 100644 --- a/gcc/config/riscv/riscv-protos.h +++ b/gcc/config/riscv/riscv-protos.h @@ -164,6 +164,7 @@ extern bool riscv_shamt_matches_mask_p (int, HOST_WIDE_INT); extern void riscv_subword_address (rtx, rtx *, rtx *, rtx *, rtx *); extern void riscv_lshift_subword (machine_mode, rtx, rtx, rtx *); extern enum memmodel riscv_union_memmodels (enum memmodel, enum memmodel); +extern bool riscv_reg_frame_related (rtx); /* Routines implemented in riscv-c.cc. */ void riscv_cpu_cpp_builtins (cpp_reader *); diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc index 680c4a728e92..38aebefa2590 100644 --- a/gcc/config/riscv/riscv.cc +++ b/gcc/config/riscv/riscv.cc @@ -6667,6 +6667,17 @@ riscv_can_eliminate (const int from ATTRIBUTE_UNUSED, const int to) return (to == HARD_FRAME_POINTER_REGNUM || to == STACK_POINTER_REGNUM); } +/* Helper to determine if reg X pertains to stack. */ +bool +riscv_reg_frame_related (rtx x) +{ + return REG_P (x) + && (REGNO (x) == FRAME_POINTER_REGNUM + || REGNO (x) == HARD_FRAME_POINTER_REGNUM + || REGNO (x) == ARG_POINTER_REGNUM + || REGNO (x) == VIRTUAL_STACK_VARS_REGNUM); +} + /* Implement INITIAL_ELIMINATION_OFFSET. FROM is either the frame pointer or argument pointer. TO is either the stack pointer or hard frame pointer. */ diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h index 817661058896..00964ccd81db 100644 --- a/gcc/config/riscv/riscv.h +++ b/gcc/config/riscv/riscv.h @@ -633,11 +633,16 @@ enum reg_class #define SUM_OF_TWO_S12_N(VALUE) \ (((VALUE) >= (-2048 * 2)) && ((VALUE) <= (-2048 - 1))) -#define SUM_OF_TWO_S12_P(VALUE) \ - (((VALUE) >= ( 2047 + 1)) && ((VALUE) <= ( 2047 * 2))) - -#define SUM_OF_TWO_S12(VALUE) \ - (SUM_OF_TWO_S12_N (VALUE) || SUM_OF_TWO_S12_P (VALUE)) +/* Variant with first value 8 byte aligned if involving stack regs. */ +#define SUM_OF_TWO_S12_P(VALUE, WANT_ALIGN) \ + ((WANT_ALIGN) \ + ? (((VALUE) >= (2032 + 1)) && ((VALUE) <= (2032 * 2))) \ + : ((VALUE >= (2047 + 1)) && ((VALUE) <= (2047 * 2)))) + +#define SUM_OF_TWO_S12(VALUE, WANT_ALIGN) \ + (SUM_OF_TWO_S12_N (VALUE) \ + || ((SUM_OF_TWO_S12_P (VALUE, false) && !(WANT_ALIGN)) \ + || (SUM_OF_TWO_S12_P (VALUE, true) && (WANT_ALIGN)))) /* If this is a single bit mask, then we can load it with bseti. Special handling of SImode 0x80000000 on RV64 is done in riscv_build_integer_1. */ diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md index 79fe861ef91f..cc8c3c653f3e 100644 --- a/gcc/config/riscv/riscv.md +++ b/gcc/config/riscv/riscv.md @@ -760,16 +760,17 @@ [(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]) + && !riscv_reg_frame_related (operands[1])" "add %0,%1,%2" - "" + "&& 1" [(set (match_dup 0) (plus:P (match_dup 1) (match_dup 3))) (set (match_dup 0) (plus:P (match_dup 0) (match_dup 4)))] { int val = INTVAL (operands[2]); - if (SUM_OF_TWO_S12_P (val)) + if (SUM_OF_TWO_S12_P (val, false)) { operands[3] = GEN_INT (2047); operands[4] = GEN_INT (val - 2047); @@ -785,6 +786,36 @@ [(set_attr "type" "arith") (set_attr "mode" "<P:MODE>")]) +(define_insn_and_split "*add<mode>3_const_sum_of_two_s12_stack" + [(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_algn" " MiA,r")))] + "riscv_reg_frame_related (operands[0]) + || riscv_reg_frame_related (operands[1])" + "add %0,%1,%2" + "&& 1" + [(set (match_dup 0) + (plus:P (match_dup 1) (match_dup 3))) + (set (match_dup 0) + (plus:P (match_dup 0) (match_dup 4)))] +{ + int val = INTVAL (operands[2]); + if (SUM_OF_TWO_S12_P (val, true)) + { + operands[3] = GEN_INT (2032); + operands[4] = GEN_INT (val - 2032); + } + else if (SUM_OF_TWO_S12_N (val)) + { + operands[3] = GEN_INT (-2048); + operands[4] = GEN_INT (val + 2048); + } + else + gcc_unreachable (); +} + [(set_attr "type" "arith") + (set_attr "mode" "<P:MODE>")]) + (define_expand "addv<mode>4" [(set (match_operand:GPR 0 "register_operand" "=r,r") (plus:GPR (match_operand:GPR 1 "register_operand" " r,r") -- 2.34.1