[PATCH] Take alignment into account for speculative loads

[Sanjoy Das]

================
Comment at: include/llvm/Analysis/ValueTracking.h:230
@@ +229,3 @@
+  bool isDereferenceableAndAlignedPointer(const Value *V, unsigned Align, 
+                                const DataLayout &DL,
+                                const Instruction *CtxI = nullptr,
----------------
The alignment (of the code, I mean :) ) is odd here -- can you please run this change through `clang-format`?

================
Comment at: lib/Analysis/Loads.cpp:147
@@ +146,3 @@
+    auto *AccessedTy = 
+      cast<PointerType>(AccessedPtr->getType())->getPointerElementType();
+    if (AccessedAlign == 0)
----------------
If you're using `getPointerElementType` then I don't think you need to `cast<PointerType>`.

================
Comment at: lib/Analysis/ValueTracking.cpp:2912
@@ +2911,3 @@
+                      unsigned Align, const DataLayout &DL) {
+  APInt BaseAlign(Offset.getBitWidth(), 0);
+  if (const AllocaInst *AI = dyn_cast<AllocaInst>(Base)) {
----------------
I'd put an `assert(isPowerOf2_32(Align))` here.

================
Comment at: lib/Analysis/ValueTracking.cpp:2923
@@ +2922,3 @@
+  
+  if (!BaseAlign.getBoolValue()){
+    Type *Ty = cast<PointerType>(Base->getType())->getPointerElementType();
----------------
Nit: spacing.  There are whitespace issues elsewhere too, please run through `clang-format`.

================
Comment at: lib/Analysis/ValueTracking.cpp:2924
@@ +2923,3 @@
+  if (!BaseAlign.getBoolValue()){
+    Type *Ty = cast<PointerType>(Base->getType())->getPointerElementType();
+    BaseAlign = DL.getABITypeAlignment(Ty);
----------------
No need to cast to `PointerType` if you're going ti call `getPointerElementType`.

================
Comment at: lib/Analysis/ValueTracking.cpp:2933
@@ +2932,3 @@
+static bool isAligned(const Value *Base, unsigned Align, const DataLayout &DL) {
+  APInt Offset(DL.getTypeStoreSizeInBits(Base->getType()), 0);
+  return isAligned(Base, Offset, Align, DL);
----------------
Consider this comment deleted -- for some reason Phabricator is unable to delete this comment.


================
Comment at: lib/Analysis/ValueTracking.cpp:2994
@@ +2993,3 @@
+    APInt Offset(DL.getPointerTypeSizeInBits(VTy), 0);
+    if (!GEP->accumulateConstantOffset(DL, Offset))
+      return false;
----------------
I think refactoring the index calculation logic to a call to `accumulateConstantOffset` should be a separate (NFC?) change.

http://reviews.llvm.org/D9791

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