On 8/9/24 05:49, Kewen.Lin wrote:
Hi,
Commit r15-2084 exposes one ICE in LRA. Firstly, before
r15-2084 KFmode has 126 bit precision while V1TImode has 128
bit precision, so the subreg (subreg:V1TI (reg:KF 131) 0) is
paradoxical_subreg_p, which stops some passes from doing
some optimization. After r15-2084, KFmode has the same mode
precision as V1TImode, passes are able to optimize more, but
it causes this ICE in LRA as described below:
For insn 106 (set (mem:V1TI ...) (subreg:V1TI (reg:KF 133) 0)),
which matches pattern
(define_insn "*vsx_le_perm_store_<mode>"
[(set (match_operand:VSX_LE_128 0 "memory_operand" "=Z,Q")
(match_operand:VSX_LE_128 1 "vsx_register_operand" "+wa,r"))]
"!BYTES_BIG_ENDIAN && TARGET_VSX && !TARGET_P9_VECTOR
&& !altivec_indexed_or_indirect_operand (operands[0], <MODE>mode)"
"@
#
#"
[(set_attr "type" "vecstore,store")
(set_attr "length" "12,8")
(set_attr "isa" "<VSisa>,*")])
LRA makes equivalence substitution on r133 with const double
(const_double:KF 0.0), selects alternative 0 and fixes up
operand 1 for constraint "wa", because operand 1 is OP_INOUT,
so it considers assigning back to it as well, that is:
lra_emit_move (type == OP_INOUT ? copy_rtx (old) : old, new_reg);
But because old has been changed to const_double in equivalence
substitution, the move is actually assigning to const_double,
which is invalid and cause ICE.
Considering reg:KF 133 is equivalent with (const_double:KF 0.0)
even though this operand is OP_INOUT, IMHO there should not be
any following uses of reg:KF 133, otherwise it doesn't have the
chance to be equivalent to (const_double:KF 0.0). From this
perspective, I think we can guard the lra_emit_move with
nonimmediate_operand to exclude such case.
Does it sound reasonable?
Yes.
btw, I also tried with disallowing equivalence substitution with
CONSTANT_P value if the corresponding operand is OP_INOUT or
OP_OUT, it can also fix this issue, but with more thinking it
seems not necessary to stop such substitution if we can handle it
later as above.
Bootstrapped and regtested on x86_64-redhat-linux and
powerpc64{,le}-linux-gnu.
Thank you for the good explanation of the problem. The patch is ok for
me. It would be nice to add a comment before `nonimmediate_operand`
that `old` can be an equivalent constant and we chose insn alternative
before the equivalent substitution.
Thank you for fixing the PR.
PR rtl-optimization/116170
gcc/ChangeLog:
* lra-constraints.cc (curr_insn_transform): Don't emit move back to
old operand if it's nonimmediate_operand.
gcc/testsuite/ChangeLog:
* gcc.target/powerpc/pr116170.c: New test.
---
gcc/lra-constraints.cc | 3 ++-
gcc/testsuite/gcc.target/powerpc/pr116170.c | 18 ++++++++++++++++++
2 files changed, 20 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr116170.c
diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index 92b343fa99a..024c85c87d9 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -4742,7 +4742,8 @@ curr_insn_transform (bool check_only_p)
}
*loc = new_reg;
if (type != OP_IN
- && find_reg_note (curr_insn, REG_UNUSED, old) == NULL_RTX)
+ && find_reg_note (curr_insn, REG_UNUSED, old) == NULL_RTX
+ && nonimmediate_operand (old, GET_MODE (old)))
{
start_sequence ();
lra_emit_move (type == OP_INOUT ? copy_rtx (old) : old, new_reg);
diff --git a/gcc/testsuite/gcc.target/powerpc/pr116170.c
b/gcc/testsuite/gcc.target/powerpc/pr116170.c
new file mode 100644
index 00000000000..6f6ca0f1ae9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr116170.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target ppc_float128_sw } */
+/* { dg-options "-mdejagnu-cpu=power8 -O2 -fstack-protector-strong
-ffloat-store" } */
+
+/* Verify there is no ICE. */
+
+int a, d;
+_Float128 b, c;
+void
+e ()
+{
+ int f = 0;
+ if (a)
+ if (b || c)
+ f = 1;
+ if (d)
+ e (f ? 0 : b);
+}
--
2.43.5
