"Robin Dapp" <[email protected]> writes:
> Hi,
>
> Similar to the outermode, now we have a case where the inner mode is
> unsplittable: (subreg:TI (concatn:V2DI ...)). We don't actually need
> it to be splittable so this patch just returns NULL if the size is
> non-constant.
>
> Bootstrapped and regtested on x86, power10, and aarch64.
> Regtested on riscv64.
It would be nice if this kind of patch submission came with more analysis.
As it stands, it feels like I have to look at the PR independently and see
whether I come to the same conclusion.
It seems that, in this case, the concatn existed before lower_subreg.
>From the jump dump:
(debug_insn 18 17 19 2 (var_location:TI v (subreg:TI (concatn:V2DI [
(and:DI (sign_extract:DI (reg:TI 134 [ _1 ])
(const_int 64 [0x40])
(const_int 0 [0]))
(sign_extract:DI (reg/v:TI 145 [ v ])
(const_int 64 [0x40])
(const_int 0 [0])))
(and:DI (sign_extract:DI (reg:TI 134 [ _1 ])
(const_int 64 [0x40])
(const_int 64 [0x40]))
(sign_extract:DI (reg/v:TI 145 [ v ])
(const_int 64 [0x40])
(const_int 64 [0x40])))
]) 0)) "foo.c":12:5 -1
(nil))
But the CONCATN handling in lower-subreg.cc is specifically for the
CONCATNs that lower-subreg.cc itself creates, rather than a general
operation. For example, other parts of the file assume that a
CONCATN has exactly two operands. There are also other asserts
that could trigger for preexisting CONCATNs.
So I think a safer fix would be to set a flag on these temporary
CONCATNs to distinguish them from preexisting CONCATNs. It looks
like volatil is free and is used in a similar fashion by var-tracking.cc
(for CONCAT rather than CONCATN).
I'd initially wondered about a hash_set instead of a flag, but that
wouldn't be robust against unsharing.
How about the attached (untested!) alternative?
Richard
gcc/
* rtl.h (rtx_def::volatil): Note use by lower-subreg.cc.
* lower-subreg.cc (DECOMPOSED_REGISTER_P): New macro.
(decompose_register): Set DECOMPOSED_REGISTER_P.
(resolve_subreg_p): Move further up file.
(resolve_reg_p): Likewise. Check DECOMPOSED_REGISTER_P.
(simplify_gen_subreg_concatn): Use resolve_reg_p and resolve_subreg_p
instead of checking directly for CONCATN.
(simplify_subreg_concatn): Likewise.
(resolve_operand_for_swap_move_operator): Likewise.
---
gcc/lower-subreg.cc | 54 +++++++++++++++++++++++++--------------------
gcc/rtl.h | 1 +
2 files changed, 31 insertions(+), 24 deletions(-)
diff --git a/gcc/lower-subreg.cc b/gcc/lower-subreg.cc
index 548f1b0d4b8..b43963f780d 100644
--- a/gcc/lower-subreg.cc
+++ b/gcc/lower-subreg.cc
@@ -76,6 +76,10 @@ along with GCC; see the file COPYING3. If not see
#define LOG_COSTS 0
#define FORCE_LOWERING 0
+/* True if a CONCATN was created by decompose_register. */
+#define DECOMPOSED_REGISTER_P(x) \
+ (RTL_FLAG_CHECK1 ("DECOMPOSED_REGISTER_P", (x), CONCATN)->volatil)
+
/* Bit N in this bitmap is set if regno N is used in a context in
which we can decompose it. */
static bitmap decomposable_context;
@@ -650,6 +654,7 @@ decompose_register (unsigned int regno)
RTVEC_ELT (v, i) = gen_reg_rtx_offset (reg, word_mode, i * UNITS_PER_WORD);
PUT_CODE (reg, CONCATN);
+ DECOMPOSED_REGISTER_P (reg) = true;
XVEC (reg, 0) = v;
if (dump_file)
@@ -661,6 +666,26 @@ decompose_register (unsigned int regno)
}
}
+/* Return whether we should resolve X into the registers into which it
+ was decomposed. */
+
+static bool
+resolve_reg_p (rtx x)
+{
+ return GET_CODE (x) == CONCATN && DECOMPOSED_REGISTER_P (x);
+}
+
+/* Return whether X is a SUBREG of a register which we need to
+ resolve. */
+
+static bool
+resolve_subreg_p (rtx x)
+{
+ if (GET_CODE (x) != SUBREG)
+ return false;
+ return resolve_reg_p (SUBREG_REG (x));
+}
+
/* Get a SUBREG of a CONCATN. */
static rtx
@@ -672,6 +697,8 @@ simplify_subreg_concatn (machine_mode outermode, rtx op,
poly_uint64 orig_byte)
unsigned int final_offset;
unsigned int byte;
+ gcc_assert (resolve_reg_p (op));
+
innermode = GET_MODE (op);
if (!interesting_mode_p (innermode, &inner_size, &inner_words))
@@ -682,7 +709,6 @@ simplify_subreg_concatn (machine_mode outermode, rtx op,
poly_uint64 orig_byte)
/* Must be constant if outer_size is. */
byte = orig_byte.to_constant ();
- gcc_assert (GET_CODE (op) == CONCATN);
gcc_assert (byte % outer_size == 0);
gcc_assert (byte < inner_size);
@@ -721,7 +747,7 @@ simplify_gen_subreg_concatn (machine_mode outermode, rtx op,
If OP is a SUBREG of a CONCATN, then it must be a simple mode
change with the same size and offset 0, or it must extract a
part. We shouldn't see anything else here. */
- if (GET_CODE (op) == SUBREG && GET_CODE (SUBREG_REG (op)) == CONCATN)
+ if (resolve_subreg_p (op))
{
rtx op2;
@@ -749,7 +775,7 @@ simplify_gen_subreg_concatn (machine_mode outermode, rtx op,
gcc_assert (innermode == GET_MODE (op));
}
- if (GET_CODE (op) == CONCATN)
+ if (resolve_reg_p (op))
return simplify_subreg_concatn (outermode, op, byte);
ret = simplify_gen_subreg (outermode, op, innermode, byte);
@@ -765,26 +791,6 @@ simplify_gen_subreg_concatn (machine_mode outermode, rtx
op,
return ret;
}
-/* Return whether we should resolve X into the registers into which it
- was decomposed. */
-
-static bool
-resolve_reg_p (rtx x)
-{
- return GET_CODE (x) == CONCATN;
-}
-
-/* Return whether X is a SUBREG of a register which we need to
- resolve. */
-
-static bool
-resolve_subreg_p (rtx x)
-{
- if (GET_CODE (x) != SUBREG)
- return false;
- return resolve_reg_p (SUBREG_REG (x));
-}
-
/* Look for SUBREGs in *LOC which need to be decomposed. */
static bool
@@ -900,7 +906,7 @@ can_decompose_p (rtx x)
static rtx
resolve_operand_for_swap_move_operator (rtx opnd)
{
- gcc_assert (GET_CODE (opnd) == CONCATN);
+ gcc_assert (resolve_reg_p (opnd));
rtx concatn = copy_rtx (opnd);
rtx op0 = XVECEXP (concatn, 0, 0);
rtx op1 = XVECEXP (concatn, 0, 1);
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 065fe8c86c7..f1314d71559 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -361,6 +361,7 @@ struct GTY((desc("0"), tag("0"),
In a PREFETCH, this flag indicates that it should be considered a
scheduling barrier.
1 in a CONCAT is VAL_NEEDS_RESOLUTION in var-tracking.cc.
+ 1 in a CONCATN is used internally by lower-subreg.cc.
Dumped as "/v" in RTL dumps. */
unsigned int volatil : 1;
/* 1 in a REG if the register is used only in exit code a loop.
--
2.55.0
