[PATCH] Commoning of target specific load/store intrinsics in Early CSE

[Sanjin Sijaric]

Thanks for the review, Pete.  New patch uploaded.


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:39
@@ -37,1 +38,3 @@
 
+/// MemIntrinsicInfo - This structure contains information about a load/store
+/// intrinsic defined by the target.
----------------
pete wrote:
> Please use \brief here instead of 'MemIntrinsicInfo -'
Changed in the updated patch.

================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:43
@@ +42,3 @@
+  MemIntrinsicInfo()
+      : ReadMem(false), WriteMem(false), MatchingId(0), PtrVal(nullptr) {}
+  bool ReadMem;
----------------
pete wrote:
> You need to initialize Vol and NumMemRefs here
Thanks for catching this.  Fixed in the updated patch.

================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:463
@@ +462,3 @@
+
+  /// \returns An instruction defining the result of a memory intrinsic.  New
+  /// instructions may be created to extract the result from an intrinsic memory
----------------
pete wrote:
> I think this comment should say it returns a value not an instruction.  I think its possible for your code to return a Value here if the st2 intrinsic was storing constants for example.
Yes, changed description in the updated patch.

================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:408
@@ +407,3 @@
+        Vol = !LI->isSimple();
+        Ptr = LI->getOperand(0);
+      } else if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
----------------
pete wrote:
> I think this can also be getPointerOperand() as you've done on the store
Yes, changed in the updated patch.

http://reviews.llvm.org/D7121

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