On 5/30/26 6:43 AM, Stefan Schulze Frielinghaus wrote:
From: Stefan Schulze Frielinghaus <[email protected]>
Currently an "entire" address is reloaded even in cases where section
anchors are involved. This makes it harder to share section anchors
which is the whole point of them. For example, in cases where
offsetable MEMs are valid do not reload .LANCHOR42+offset but only
.LANCHOR42 and replace the address with the resulting reload register
and the offset. As a consequence subsequent passes only have to deal
with register equivalences in order to share section anchors. For
example, consider testsuite/gcc.target/s390/section-anchors-4.c.
Without this patch, after LRA we end up with
20: %r1:DI=`*.LANCHOR0'
17: %f0:DF=[%r1:DI]
19: %r1:DI=const(`*.LANCHOR0'+0x8)
12: {%f0:DF=%f0:DF+[%r1:DI];clobber %cc:CC;}
and with this patch
20: %r1:DI=`*.LANCHOR0'
17: %f0:DF=[%r1:DI]
19: %r1:DI=`*.LANCHOR0'
12: {%f0:DF=%f0:DF+[%r1:DI+0x8];clobber %cc:CC;}
In insn 19 only the section anchor is reloaded. This allows postreload
to remove the redundant load such that we end up with
20: %r1:DI=`*.LANCHOR0'
17: %f0:DF=[%r1:DI]
12: {%f0:DF=%f0:DF+[%r1:DI+0x8];clobber %cc:CC;}
gcc/ChangeLog:
* lra-constraints.cc (reload_section_anchor_p): New function.
(curr_insn_transform): For offsetable MEMs, try reloading the
section anchor only.
gcc/testsuite/ChangeLog:
* gcc.target/s390/section-anchors-4.c: New test.
---
Notes:
Bootstrapped and regtested on
- aarch64-unknown-linux-gnu
- powerpc64le-unknown-linux-gnu
- s390x-ibm-linux-gnu
- x86_64-pc-linux-gnu
Ok for mainline?
Yes with minor changes mentioned below. You can address mem subreg
operand (it is just a missed optimization opportunity) in next patch if
you want.
Thank you for your patch.
To add some more evidence why this patch might be beneficial here and
there. Using a work-in-progress patch I see sequences in SPEC lbm
benchmark as e.g.
larl %r5,.LANCHOR0+128
vleg %v22,0(%r5),0
larl %r5,.LANCHOR0+136
ld %f4,0(%r5)
larl %r5,.LANCHOR0+144
vleg %v21,0(%r5),0
larl %r5,.LANCHOR0+152
ld %f8,0(%r5)
larl %r5,.LANCHOR0+160
ld %f12,0(%r5)
larl %r5,.LANCHOR0+168
ld %f14,0(%r5)
larl %r5,.LANCHOR0+176
ld %f9,0(%r5)
larl %r5,.LANCHOR0+184
ld %f11,0(%r5)
larl %r5,.LANCHOR0+192
vleg %v17,0(%r5),0
and with this patch
larl %r5,.LANCHOR0
ld %f4,136(%r5)
ld %f8,152(%r5)
vleg %v22,128(%r5),0
vleg %v21,144(%r5),0
vleg %v17,192(%r5),0
ld %f12,160(%r5)
ld %f14,168(%r5)
ld %f9,176(%r5)
ld %f11,184(%r5)
where all the LARLs are folded into one. Certainly a worst case example
but I see similar usages here and there.
gcc/lra-constraints.cc | 77 +++++++++++++++++++
.../gcc.target/s390/section-anchors-4.c | 24 ++++++
2 files changed, 101 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/s390/section-anchors-4.c
diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index d6056cf7acc..aee94891dc3 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -4315,6 +4315,58 @@ postpone_insns (rtx_insn *first)
}
}
+/* Test whether the n-th operand is a MEM where the address is the sum of a
+ section anchor and a constant and return true in case of reloading the
+ section anchor only results in a satisfiable operand w.r.t. its
+ corresponding constraint. Otherwise return false. */
+
+static bool
+reload_section_anchor_p (int nop)
+{
+ rtx op = *curr_id->operand_loc[nop];
+ if (!MEM_P (op))
+ return false;
It would be nice to process subreg of mem here too. But you can make a
patch for this later.
+ rtx addr = XEXP (op, 0);
+
+ if (GET_CODE (addr) != CONST
+ || GET_CODE (XEXP (addr, 0)) != PLUS
+ || GET_CODE (XEXP (XEXP (addr, 0), 0)) != SYMBOL_REF
+ || !SYMBOL_REF_ANCHOR_P (XEXP (XEXP (addr, 0), 0))
+ || !CONST_INT_P (XEXP (XEXP (addr, 0), 1))
+ /* Some offsets are valid in conjunction with a symbol and
+ invalid in conjunction with a register. Thus, pull out
+ the anchor only in case the offset is a valid anchor
+ offset. */
+ || INTVAL (XEXP (XEXP (addr, 0), 1)) < targetm.min_anchor_offset
+ || INTVAL (XEXP (XEXP (addr, 0), 1)) > targetm.max_anchor_offset)
+ return false;
+
+ /* Now test whether a new address of the form REG+DISPLACEMENT is valid for
+ the selected alternative. In order to do so, utilize lra_pmode_pseudo
+ instead of an actual reload register. */
+
+ rtx offset = XEXP (XEXP (addr, 0), 1);
+ rtx new_addr = gen_rtx_PLUS (Pmode, lra_pmode_pseudo, offset);
+ rtx new_op = shallow_copy_rtx (op);
+ XEXP (new_op, 0) = new_addr;
+
+ /* Skip alternatives before the one requested. */
Comment a bit misleading. Skipping means some checking previous
alternatives, e.g. in a loop. I'd say "Get operand constraints for
given alternative".
+ const char *p = (curr_static_id->operand_alternative
+ [goal_alt_number * curr_static_id->n_operands + nop]
+ .constraint);
+ char c;
+ for (;
+ (c = *p) && c != ',' && c != '#';
+ p += CONSTRAINT_LEN (c, p))
+ {
+ enum constraint_num cn = lookup_constraint (p);
+ if (constraint_satisfied_p (new_op, cn))
+ return true;
+ }
+
+ return false;
+}
+
/* Main entry point of the constraint code: search the body of the
current insn to choose the best alternative. It is mimicking insn
alternative cost calculation model of former reload pass. That is
@@ -4882,6 +4934,31 @@ curr_insn_transform (bool check_only_p)
new_reg = emit_inc (rclass, *loc,
/* This value does not matter for MODIFY. */
GET_MODE_SIZE (GET_MODE (op)));
+ /* Try to pull out section anchors. For example, instead of
+ reloading an "entire" address like .LANCHOR42+offset only reload
+ .LANCHOR42 and use the new reload register as the base register.
+ This allows following optimizations to share section anchors and
+ remove redundant loads. */
+ else if (reload_section_anchor_p (i))
+ {
+ rtx anchor = XEXP (XEXP (*loc, 0), 0);
+ rtx offset = XEXP (XEXP (*loc, 0), 1);
+
+ enum reg_class rclass
+ = base_reg_class (GET_MODE (op), MEM_ADDR_SPACE (op), MEM,
+ SCRATCH, curr_insn);
+
I see exactly the same definition and value of rclass a bit above (not
seen in this diff context). So it is redundant.
+ if (get_reload_reg (OP_IN, Pmode, anchor, rclass, NULL, false,
+ false, "offsetable address", &new_reg))
+ lra_emit_move (new_reg, anchor);
+
+ rtx new_addr = gen_rtx_PLUS (Pmode, new_reg, offset);
+ rtx new_op = copy_rtx (op);
I think it should be a shallow_copy_rtx to as in
reload_section_anchor_p. It is harmless but make GC work more.
+ XEXP (new_op, 0) = new_addr;
+
+ new_reg = new_op;
+ loc = curr_id->operand_loc[i];
+ }
else if (get_reload_reg (OP_IN, Pmode, *loc, rclass,
NULL, false, false,
"offsetable address", &new_reg))
diff --git a/gcc/testsuite/gcc.target/s390/section-anchors-4.c
b/gcc/testsuite/gcc.target/s390/section-anchors-4.c
new file mode 100644
index 00000000000..9938c057437
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/section-anchors-4.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=z13" } */
+/* { dg-final { scan-assembler-times "\tlarl\t" 1 } } */
+
+/* Prior LRA we have
+
+ 12: {%f0:DF=[`*.LANCHOR0']+[const(`*.LANCHOR0'+0x8)];clobber %cc:CC;}
+
+ and afterwards
+
+ 20: %r1:DI=`*.LANCHOR0'
+ 17: %f0:DF=[%r1:DI]
+ 19: %r1:DI=`*.LANCHOR0'
+ 12: {%f0:DF=%f0:DF+[%r1:DI+0x8];clobber %cc:CC;}
+
+ where postreload removes the redundant insn 19. */
+
+double x, y;
+
+double
+test ()
+{
+ return x + y;
+}
