On 7/14/2026 1:36 AM, wangjue wrote:
From: "wangjue.wangjue" <[email protected]>
riscv_legitimize_address handles scaled indexed frame addresses specially
when the constant offset fits an I-type immediate. With a large offset,
generic address legalization can materialize the same frame base at each
reference.
Form the frame base before adding the scaled index for both small and large
offsets. Keeping a large frame base in a pseudo makes it available to CSE
and avoids repeated materialization.
gcc/ChangeLog:
* config/riscv/riscv.cc (riscv_legitimize_address): Handle large
constant offsets in scaled indexed frame addresses.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/large-frame-indexed-base.c: New test.
* gcc.target/riscv/large-frame-indexed-base-2.c: New test.
Note this is more general than just FP related addresses. The variable
name in that routine isn't great in that regard as it would seem to
imply this code is only used on frame pointer based addresses. I think
this is a meaningful issue and thanks for identifying it! WHen we
discussed it in the patchwork call today, Palmer indicated (and I
agreed) that there's likely several improvements that could be made for
cases where the stack/frame is large leading to offsets that don't fit.
While that code has meaningfully improved over the last year, there's
almost certainly further improvements that could be made.
@@ -3479,10 +3480,17 @@ riscv_legitimize_address (rtx x, rtx oldx
ATTRIBUTE_UNUSED,
if (GET_CODE (index) == MULT)
shift_val = exact_log2 (shift_val);
- rtx reg1 = gen_reg_rtx (Pmode);
- rtx reg2 = gen_reg_rtx (Pmode);
- rtx reg3 = gen_reg_rtx (Pmode);
- riscv_emit_binary (PLUS, reg1, fp, GEN_INT (offset));
+ rtx reg1, reg2, reg3;
+ if (SMALL_OPERAND (offset))
+ {
+ reg1 = gen_reg_rtx (Pmode);
+ riscv_emit_binary (PLUS, reg1, fp, GEN_INT (offset));
+ }
+ else
+ /* Expose FP + OFFSET as a CSE candidate. */
+ reg1 = force_reg (Pmode, riscv_add_offset (NULL, fp, offset));
+ reg2 = gen_reg_rtx (Pmode);
+ reg3 = gen_reg_rtx (Pmode);
riscv_emit_binary (ASHIFT, reg2, XEXP (index, 0), GEN_INT
(shift_val));
riscv_emit_binary (PLUS, reg3, reg2, reg1);
Note that riscv_add_offset handles SMALL_OPERAND just fine. So it seems
like we could call it unconditionally rather than having separate paths
for SMALL_OPERAND and !SMALL_OPERAND in this code.
So rather than the conditional, I think you can just do something like
this to cover both cases:
reg1 = force_reg (Pmode, riscv_add_offset (NULL, fp, offset));
Can you try that?
Can you confirm that you're submitting this under either a pre-existing
copyright assignment from Alibaba or under the DCO? It may not be
strictly necessary after making the change above (since the patch
becomes trivial), but best to make sure we've got the legal basis covered.
Finally, how was this tested?
jeff
