Re: [PATCH] Fix CSE volatile MEM handling (PR rtl-optimization/64756)

2015-02-03 Thread Eric Botcazou
 2015-02-02  Jakub Jelinek  ja...@redhat.com
 
   PR rtl-optimization/64756
   * cse.c (cse_insn): If dest != SET_DEST (sets[i].rtl) and
   HASH (SET_DEST (sets[i].rtl), mode) computation sets do_not_record,
   invalidate and do not record it.
 
   * gcc.c-torture/execute/pr64756.c: New test.

OK if you factor out the common code with the similar block for 'dest' above 
into an invalidate_dest function (note that the MEM_P case is identical to the 
REG_P and SUBREG cases) and invoke it on 'dest' and SET_DEST (sets[i].rtl).

-- 
Eric Botcazou


[PATCH] Fix CSE volatile MEM handling (PR rtl-optimization/64756)

2015-02-02 Thread Jakub Jelinek
Hi!

We miscompile the following testcase, because first we add
a mem/v into the hash table (which should not happen), later on
during merge_equiv_classes a new element for that mem/v is added
and doesn't even have in_memory set (because HASH failed with do_not_record
but nothing checked it) and later on this results in that memory not being
properly invalidated.

The initial problem is that if SET_DEST (sets[i].rtl) is a volatile mem,
but we have a known value at that memory, we compute initially
sets[i].dest_hash as hash value of the known value.  That doesn't result
into do_not_record, and when we recompute actual hash value for the MEM,
we ignore the do_not_record flag.

I've tested it also with logging when did this trigger, and in both
bootstraps it triggered only on the new testcase and in 64-bit build of
libstdc++-v3/testsuite/29_atomics/atomic_flag/test_and_set/explicit-hle.cc.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2015-02-02  Jakub Jelinek  ja...@redhat.com

PR rtl-optimization/64756
* cse.c (cse_insn): If dest != SET_DEST (sets[i].rtl) and
HASH (SET_DEST (sets[i].rtl), mode) computation sets do_not_record,
invalidate and do not record it.

* gcc.c-torture/execute/pr64756.c: New test.

--- gcc/cse.c.jj2015-01-23 20:49:11.0 +0100
+++ gcc/cse.c   2015-02-02 11:51:57.508084360 +0100
@@ -5521,7 +5521,22 @@ cse_insn (rtx_insn *insn)
}
 
   if (sets[i].rtl != 0  dest != SET_DEST (sets[i].rtl))
-   sets[i].dest_hash = HASH (SET_DEST (sets[i].rtl), mode);
+   {
+ do_not_record = 0;
+ sets[i].dest_hash = HASH (SET_DEST (sets[i].rtl), mode);
+ if (do_not_record)
+   {
+ rtx dst = SET_DEST (sets[i].rtl);
+ if (REG_P (dst) || GET_CODE (dst) == SUBREG)
+   invalidate (dst, VOIDmode);
+ else if (MEM_P (dst))
+   invalidate (dst, VOIDmode);
+ else if (GET_CODE (dst) == STRICT_LOW_PART
+  || GET_CODE (dst) == ZERO_EXTRACT)
+   invalidate (XEXP (dst, 0), GET_MODE (dst));
+ sets[i].rtl = 0;
+   }
+   }
 
 #ifdef HAVE_cc0
   /* If setting CC0, record what it was set to, or a constant, if it
--- gcc/testsuite/gcc.c-torture/execute/pr64756.c.jj2015-02-02 
11:53:06.903882851 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr64756.c   2015-02-02 
11:52:53.0 +0100
@@ -0,0 +1,30 @@
+/* PR rtl-optimization/64756 */
+
+int a, *tmp, **c = tmp;
+volatile int d;
+static int *volatile *e = tmp;
+unsigned int f;
+
+static void
+fn1 (int *p)
+{
+  int g;
+  for (; f  1; f++)
+for (g = 1; g = 0; g--)
+  {
+   d || d;
+   *c = p;
+
+   if (tmp != a)
+ __builtin_abort ();
+
+   *e = 0;
+  }
+}
+
+int
+main ()
+{
+  fn1 (a);
+  return 0;
+}

Jakub