On Sat, 2020-02-08 at 10:41 -0600, Segher Boessenkool wrote:
> On Fri, Feb 07, 2020 at 09:00:40AM -0700, Jeff Law wrote:
> > On Thu, 2020-02-06 at 07:56 -0600, Segher Boessenkool wrote:
> > > On Wed, Feb 05, 2020 at 11:48:23AM -0700, Jeff Law wrote:
> > > > Yea, it's closely related. In your case you need to effectively ignore
> > > > the nop insn to get the optimization you want. In mine that nop insn
> > > > causes an ICE.
> > > >
> > > > I think we could take your cse bits + adding a !CALL_P separately from
> > > > the simplify-rtx stuff which Segher objected to. THat'd likely solve
> > > > the ARM ICEs and take you a tiny step forward on optimizing that SVE
> > > > case. Thoughts?
> > >
> > > CSE should consistently keep track of what insns are no-op moves (in its
> > > definition, all passes have a slightly different definition of this),
> > > and use that everywhere consistently.
> > So does that mean you object to the cse.c portion of Richard's patch?
>
> It's more a "what we need to do in the future" thing, it is stage 4 now,
> it is too big a change to do now.
I suspect you're referring to the simplify-rtx bits from his patch which I agree
are not appropriate for stage4. The cse bits from that patch are are simple.
>
> What patch? The "34" patch? https://gcc.gnu.org/r278411 .
>
> I don't think each stanza of code should use it's own "noop-ness", no.
Richard's patch is actually better than mine in that regard as it handles mem
and
reg nop moves in an indentical way. I don't think refactoring the cse.c code is
advisable now though -- but I do want to fix the multiply-reported ICE on ARM
and
Richard's cse changes are the cleanest way to do that that I can see.
>
> I don't know if this patch makes matters worse or not. It doesn't seem
> suitable for stage 4 though. And Richard said the cse.c part breaks
> rs6000, if that is true, yes I do object ;-)
The rs6000 port breakage is trivial to fix. In fact, I did so and ran it
through
my tester, which includes ppc64le and ppc64 a slew of *-elf targets x86 native
and more.
Concretely I'm proposing the following patch to address 90275 and its
duplicates.
PR rtl-optimization/90275
* cse.c (cse_insn): Delete no-op register moves too.
PR rtl-optimization/90275
* gcc.c-torture/compile/pr90275.c: New test.
diff --git a/gcc/cse.c b/gcc/cse.c
index 79ee0ce80e3..1779bb9a333 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -4625,7 +4625,7 @@ cse_insn (rtx_insn *insn)
for (i = 0; i < n_sets; i++)
{
bool repeat = false;
- bool mem_noop_insn = false;
+ bool noop_insn = false;
rtx src, dest;
rtx src_folded;
struct table_elt *elt = 0, *p;
@@ -5324,9 +5324,11 @@ cse_insn (rtx_insn *insn)
}
/* Similarly, lots of targets don't allow no-op
- (set (mem x) (mem x)) moves. */
+ (set (mem x) (mem x)) moves. Even (set (reg x) (reg x))
+ might be impossible for certain registers (like CC registers). */
else if (n_sets == 1
- && MEM_P (trial)
+ && ! CALL_P (insn)
+ && (MEM_P (trial) || REG_P (trial))
&& MEM_P (dest)
&& rtx_equal_p (trial, dest)
&& !side_effects_p (dest)
@@ -5334,7 +5336,7 @@ cse_insn (rtx_insn *insn)
|| insn_nothrow_p (insn)))
{
SET_SRC (sets[i].rtl) = trial;
- mem_noop_insn = true;
+ noop_insn = true;
break;
}
@@ -5562,8 +5564,8 @@ cse_insn (rtx_insn *insn)
sets[i].rtl = 0;
}
- /* Similarly for no-op MEM moves. */
- else if (mem_noop_insn)
+ /* Similarly for no-op moves. */
+ else if (noop_insn)
{
if (cfun->can_throw_non_call_exceptions && can_throw_internal (insn))
cse_cfg_altered = true;
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr90275.c
b/gcc/testsuite/gcc.c-torture/compile/pr90275.c
new file mode 100644
index 00000000000..83e0df77226
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr90275.c
@@ -0,0 +1,27 @@
+a, b, c;
+
+long long d;
+
+e() {
+
+ char f;
+
+ for (;;) {
+
+ c = a = c ? 5 : 0;
+
+ if (f) {
+
+ b = a;
+
+ f = d;
+
+ }
+
+ (d || b) < (a > e) ?: (b ? 0 : f) || (d -= f);
+
+ }
+
+}
+
+
