[PATCH] Take alignment into account for speculative loads

[Matthias Braun]

Generally looks fine to me, but I better leave the final judgement to someone more familiar with this part of the code.

Some comments:


================
Comment at: include/llvm/Analysis/ValueTracking.h:231
@@ +230,3 @@
+
+  /// isDereferenceablePointer - Return true if this is always a dereferenceable
+  /// pointer with alignment greater or equal than requested. If the context
----------------
Don't repeat the method name in the documentation comment, this practice is in widespread use in llvm but discouraged.

================
Comment at: lib/Analysis/Loads.cpp:146
@@ +145,3 @@
+
+    auto *AccessedTy = AccessedPtr->getType()->getPointerElementType();
+    if (AccessedAlign == 0)
----------------
"Type *AccessedTy" would be clearer to the reader.

================
Comment at: lib/Analysis/MemDerefPrinter.cpp:60-62
@@ -57,3 +59,5 @@
       if (isDereferenceablePointer(PO, DL))
-        Vec.push_back(PO);
+        Deref.push_back(PO);
+      if (isDereferenceableAndAlignedPointer(PO, LI->getAlignment(), DL))
+        DerefAndAligned.push_back(PO);
     }
----------------
So you print dereferenceableAndAligned pointers twice? It would probably look nicer if you only print each value, you could still indicate the alignment by printing something like " (unaligned)" behind unaligned pointers.

================
Comment at: lib/Analysis/MemDerefPrinter.cpp:70
@@ -65,2 +69,3 @@
   OS << "The following are dereferenceable:\n";
-  for (auto &V: Vec) {
+  for (auto &V: Deref) {
+    V->print(OS);
----------------
"Value *" would be more clear for the reader and that makes it obvious that the reference is unnecessary here. Same in the following loop.

================
Comment at: lib/Analysis/ValueTracking.cpp:2943
@@ +2942,3 @@
+  APInt Alignment(Offset.getBitWidth(), Align);
+  return BaseAlign.uge(Alignment) && Offset.urem(Alignment) == 0;
+}
----------------
The second part could be slightly optimized (although I wish we had a method named APInt::isZero()):
assert(Alignment.isPowerOf2());
return ... && !(Offset & (Alignment-1)).getBoolValue();

================
Comment at: lib/Analysis/ValueTracking.cpp:2960
@@ -2935,2 +2959,3 @@
 
   // These are obviously ok.
+  if (isa<AllocaInst>(V))
----------------
maybe add an "if aligned".

http://reviews.llvm.org/D9791

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