[PATCH] D26224: NewGVN

[Daniel Berlin via llvm-commits]

dberlin added inline comments.


================
Comment at: lib/Transforms/Scalar/NewGVN.cpp:1001
+    }
+    markUsersTouched(V);
+  }
----------------
There's a bug (well, incompleteness)  here that i just noticed
For memory, we also need to mark the uses of the MemoryDef/MemoryUse for the instruction as touched.
(and handle MemoryPhi's).
While most of the time, they are already touched, otherwise, will not iterate when we have discovered something about memory for, say, store over store.

MemoryPhi's will need value numbering when I fix the store over store problem, but for now, i'd just skip them.

So i'd add something like
  if (MemoryAccess *MA = MSSA->getMemoryAccess(V)) 
    markMemoryUsersTouched(MA);

where markMemoryUsersTouched just walks the users of MA and mark MA->getInst() touched iff it's a MemoryUseOrDef.

This will ensure memory instructions change when we discover new things about them.

Sorry, in GCC, they are part of the IR, so there aren't two use lists :)



https://reviews.llvm.org/D26224