[PATCH] D10749: DAGCombiner: Assume invariant load cannot alias a store

[hfinkel at anl.gov]

hfinkel added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13821
@@ +13820,3 @@
+  // they cannot alias.
+  if (Op0->isInvariant() && Op1->writeMem())
+    return false;
----------------
arsenm wrote:
> reames wrote:
> > Not sure about this, but is writeMem correct here?  At least most places, we use a conservative notion for writes.  i.e. it *may* write, not it *must* write.  If we had an instruction which may write, but actually just reads in this context, the aliasing could be wrong.  
> > 
> > !mayLoad might be a possibility.
> The mayLoad/mayStore are properties on a MachineInstr, but these are DAG nodes which I don't think have the same concept.
The situation here is somewhat subtle. writeMem delegates to MachineMemOperand::isStore(), which checks whether the MOStore flag is set. What has this flag set? For the most part, just actual stores (trunc stores, atomic stores, etc.). However, this does include the MMOs on masked stores (which might or might not actually store anything), and also include all memory intrinsic nodes that might write to memory. The intrinsics include things like ISD::PREFETCH (which is tagged as writing for dependency reasons, but never writes anything). In short, care is required. But I think this does not hurt anything assuming you special-case ISD::PREFETCH here, so please do that.




http://reviews.llvm.org/D10749