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

Matt Arsenault Matthew.Arsenault at amd.com
Fri Jun 26 12:00:00 PDT 2015


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13821
@@ +13820,3 @@
+  // they cannot alias.
+  if (Op0->isInvariant() && Op1->writeMem())
+    return false;
----------------
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.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13832
@@ -13823,3 +13831,3 @@
   const void *CV1, *CV2;
   bool isFrameIndex1 = FindBaseOffset(Op0->getBasePtr(),
                                       Base1, Offset1, GV1, CV1);
----------------
reames wrote:
> Is this case handled by the UseAA option below?  I'd really expect it to be.  Is enabling that for your target an option?
That's actually the source of the set of problems I'm working on now.

The short version is enabling AA results in worse code partially because of this check. 

UseAA enables trying to use FindBetterChain to try to move loads and stores to the same chain, but it doesn't work very well. Pretty much all loads are dependent on the constant kernel argument load, so this fix allows it to work sometimes. However, I run into more problems from that because both the existing code in MergeConsecutiveStores assumes stores are on consecutive chains, and FindBetterChain doesn't do a great job at finding the same chain for all of the adjacent stores.

http://reviews.llvm.org/D10749

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list