From: Vineet Gupta <vineetg@rivosinc.com>
To: gcc-patches@gcc.gnu.org
Cc: Jeff Law <jeffreyalaw@gmail.com>,
kito.cheng@gmail.com, Palmer Dabbelt <palmer@rivosinc.com>,
gnu-toolchain@rivosinc.com, Robin Dapp <rdapp.gcc@gmail.com>,
Vineet Gupta <vineetg@rivosinc.com>
Subject: [gcc-15 2/3] RISC-V: avoid LUI based const mat: keep stack offsets aligned
Date: Sat, 16 Mar 2024 10:35:23 -0700 [thread overview]
Message-ID: <20240316173524.1147760-3-vineetg@rivosinc.com> (raw)
In-Reply-To: <20240316173524.1147760-1-vineetg@rivosinc.com>
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 <vineetg@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
next prev parent reply other threads:[~2024-03-16 17:35 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-16 17:35 [gcc-15 0/3] RISC-V improve stack/array access by constant mat tweak Vineet Gupta
2024-03-16 17:35 ` [gcc-15 1/3] RISC-V: avoid LUI based const materialization ... [part of PR/106265] Vineet Gupta
2024-03-16 20:28 ` Jeff Law
2024-03-19 0:07 ` Vineet Gupta
2024-03-23 5:59 ` Jeff Law
2024-03-16 17:35 ` Vineet Gupta [this message]
2024-03-16 20:21 ` [gcc-15 2/3] RISC-V: avoid LUI based const mat: keep stack offsets aligned Jeff Law
2024-03-19 0:27 ` Vineet Gupta
2024-03-19 6:48 ` Andrew Waterman
2024-03-19 13:10 ` Jeff Law
2024-03-19 20:05 ` Vineet Gupta
2024-03-19 20:58 ` Andrew Waterman
2024-03-19 21:17 ` Palmer Dabbelt
2024-03-20 18:57 ` Jeff Law
2024-03-23 6:05 ` Jeff Law
2024-03-16 17:35 ` [gcc-15 3/3] RISC-V: avoid LUI based const mat in prologue/epilogue expansion [PR/105733] Vineet Gupta
2024-03-16 20:27 ` Jeff Law
2024-03-19 4:41 ` [gcc-15 0/3] RISC-V improve stack/array access by constant mat tweak Jeff Law
2024-03-21 0:45 ` Vineet Gupta
2024-03-21 14:36 ` scheduler queue flush (was Re: [gcc-15 0/3] RISC-V improve stack/array access by constant mat tweak) Vineet Gupta
2024-03-21 14:45 ` Jeff Law
2024-03-21 17:19 ` Vineet Gupta
2024-03-21 19:56 ` Jeff Law
2024-03-22 0:34 ` scheduler queue flush Vineet Gupta
2024-03-22 8:47 ` scheduler queue flush (was Re: [gcc-15 0/3] RISC-V improve stack/array access by constant mat tweak) Richard Biener
2024-03-22 12:29 ` Jeff Law
2024-03-22 16:56 ` Vineet Gupta
2024-03-25 3:05 ` Jeff Law
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240316173524.1147760-3-vineetg@rivosinc.com \
--to=vineetg@rivosinc.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=gnu-toolchain@rivosinc.com \
--cc=jeffreyalaw@gmail.com \
--cc=kito.cheng@gmail.com \
--cc=palmer@rivosinc.com \
--cc=rdapp.gcc@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).