On 5/11/2026 2:19 AM, Zhongyao Chen wrote:
modified_between_p and modified_in_p conservatively treat any write to
a register as conflicting with all its subregs, even disjoint ones.
This blocks combine optimizations and leaves redundant register moves.
The issue is visible on RISC-V with -mrvv-vector-bits=zvl and may
affect other architectures.
Use chunk-based analysis to recognize disjoint subreg writes as
non-conflicting. Extract implicit_reg_set_p from reg_set_p to
separate implicit clobber checks from explicit subreg write analysis.
For example, with RISC-V VLS:
Before:
vfadd.vf v1,v15,fa4
vfadd.vf v8,v8,fa5
vmv1r.v v15,v1 # Redundant move
After:
vfadd.vf v8,v8,fa4
vfadd.vf v15,v15,fa5 # Direct operation, no move needed
Changes from v1 (following Richard's comments):
- Switch to `rtx_properties` (with a new `ignore_srcs` flag) to avoid
re-implementing instruction traversal logic.
- Drop the strict mode size check; the chunk-based analysis is already
robust enough for general cases.
- Use `read_modify_subreg_p` to filter out full-register writes early
and remove redundant precondition checks.
Thanks for Richard's helpful and detailed guidance.
gcc/
* rtlanal.h (class rtx_properties): Add ignore_srcs flag.
(rtx_properties::try_to_add_src_1): New prototype.
(rtx_properties::try_to_add_src): Inline wrapper around
try_to_add_src_1.
* rtlanal.cc (rtx_properties::try_to_add_src_1): Rename from
try_to_add_src.
(implicit_reg_set_p): New function.
(reg_set_p): Refactored to call implicit_reg_set_p.
(subreg_write_clobbers_ref_p): New function.
(subreg_modified_by_dest_p): New function.
(subreg_modified_in_p): New function.
(subreg_modified_between_p): New function.
(modified_between_p): Use read_modify_subreg_p up-front and
subreg_modified_between_p.
(modified_in_p): Ditto.
gcc/testsuite/
* gcc.target/riscv/rvv/base/tuple-zvl-subreg.c: New test.
Signed-off-by: Zhongyao Chen <[email protected]>
---
gcc/rtlanal.cc | 188 +++++++++++++++++-
gcc/rtlanal.h | 17 +-
.../riscv/rvv/base/tuple-zvl-subreg.c | 33 +++
3 files changed, 232 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/tuple-zvl-subreg.c
diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc
index 88561a54e5a..f79d62371d5 100644
--- a/gcc/rtlanal.cc
+++ b/gcc/rtlanal.cc
+/* Return true if storing to DEST changes the value of SUBREG_REF, where both
+ are simple subregs of the same register with a known common containing size.
+
+ A subreg store can clobber the whole REGMODE_NATURAL_SIZE chunk that
+ contains it, even when the stored mode itself is smaller. */
+
+static bool
+subreg_write_clobbers_ref_p (const_rtx subreg_ref, const_rtx dest)
+{
+ poly_uint64 chunk_size = REGMODE_NATURAL_SIZE (GET_MODE (SUBREG_REG (dest)));
+ poly_uint64 ref_chunk_size
+ = REGMODE_NATURAL_SIZE (GET_MODE (SUBREG_REG (subreg_ref)));
+ if (maybe_gt (ref_chunk_size, chunk_size))
+ chunk_size = ref_chunk_size;
+ poly_uint64 container_size = GET_MODE_SIZE (GET_MODE (SUBREG_REG (dest)));
+ HOST_WIDE_INT num_chunks;
+ if (!constant_multiple_p (container_size, chunk_size, &num_chunks))
+ return true;
+
+ poly_uint64 ref_start = SUBREG_BYTE (subreg_ref);
+ poly_uint64 ref_size = GET_MODE_SIZE (GET_MODE (subreg_ref));
+ poly_uint64 dest_start = SUBREG_BYTE (dest);
+ poly_uint64 dest_size = GET_MODE_SIZE (GET_MODE (dest));
+
+ for (HOST_WIDE_INT i = 0; i < num_chunks; ++i)
+ {
+ poly_uint64 chunk_start = i * chunk_size;
+ if (ranges_maybe_overlap_p (chunk_start, chunk_size,
+ dest_start, dest_size)
+ && ranges_maybe_overlap_p (chunk_start, chunk_size,
+ ref_start, ref_size))
+ return true;
+ }
+ return false;
+}
+
+/* Return true if DEST modifies SUBREG_REF. */
+
+static bool
+subreg_modified_by_dest_p (const_rtx subreg_ref, const_rtx dest)
+{
+ if (!dest)
+ return false;
+
+ switch (GET_CODE (dest))
+ {
+ case SUBREG:
+ if (REG_P (SUBREG_REG (dest))
+ && REGNO (SUBREG_REG (subreg_ref)) == REGNO (SUBREG_REG (dest)))
+ return subreg_write_clobbers_ref_p (subreg_ref, dest);
+ return reg_overlap_mentioned_p (subreg_ref, dest);
+
+ case STRICT_LOW_PART:
+ case ZERO_EXTRACT:
+ case SIGN_EXTRACT:
+ return subreg_modified_by_dest_p (subreg_ref, XEXP (dest, 0));
So it's not a correctness thing but STRICT_LOW_PART is known to not
change bits outside the mode specified by the STRICT_LOW_PART. We
don't use STRICT_LOW_PART a lot, but I think it describes at least some
of the semantics you want. Similarly for ZERO_EXTRACT. SIGN_EXTRACT
isn't supposed to ever be an lvalue IIRC.
+
+ case REG:
+ case SCRATCH:
+ case PC:
+ return reg_overlap_mentioned_p (subreg_ref, dest);
+
+ case PARALLEL:
+ for (int i = XVECLEN (dest, 0) - 1; i >= 0; --i)
+ if (XEXP (XVECEXP (dest, 0, i), 0)
+ && subreg_modified_by_dest_p (subreg_ref,
+ XEXP (XVECEXP (dest, 0, i), 0)))
Is this really the right value to pass for the recursive step?
Specifically why extract XEXP (..., 0) for the recursive call? Usually
we'd just pass XVECEXP. It also seems like this is going to consider a
USE as modification, all the more reason not to extract the XEXP (...,
0) -- you need to know if you're dealing with a USE or a CLOBBER. The
former doesn't change anything, it's just a reference. The latter does
change things, obviously.
+
+/* Return true if X, a simple partial SUBREG of a REG, is modified in INSN. */
+
+static bool
+subreg_modified_in_p (const_rtx x, const_rtx insn)
Do you have to worry about embedded side effects here? For example
PRE_INC, POST_INC, PRE_MODIFY, POST_MODIFY?
+{
+ gcc_checking_assert (SUBREG_P (x) && REG_P (SUBREG_REG (x)));
+
+ if (implicit_reg_set_p (SUBREG_REG (x), insn))
+ return true;
+
+ vec_rtx_properties properties;
+ properties.ignore_srcs = true;
+ if (INSN_P (insn))
+ properties.try_to_add_insn (as_a <const rtx_insn *> (insn), false);
+ else
+ properties.try_to_add_pattern (insn);
+ bool possible_conflict = false;
+ for (auto ref : properties.refs ())
+ if (ref.is_reg () && ref.is_write ()
+ && ref.regno == REGNO (SUBREG_REG (x)))
+ {
+ if (!ref.in_subreg ())
+ return true;
+ possible_conflict = true;
+ }
This seems overly simplistic. Don't we have more cases where this could
overlap? The REGNO's don't necessarily have to match to have an
overlap. Consider an LMUL8 starting at v0, it's going to write v0, v1,
v2, v3, v4, v5, v5, v7. It would overlap with a narrower subreg within
that range, even though the lowest register number doesn't match.
I suspect this code may not be correct for big endian cases as well.
In general, I like what you're trying to do, but I'm really struggling
to see how it's correct right now.
Richard S. Have you done a deep dive on this code?
jeff
