[PATCH] Take alignment into account for speculative loads
Sanjoy Das
sanjoy at playingwithpointers.com
Wed Jun 3 19:09:30 PDT 2015
Some comments inline.
This change is still too large, do you think it makes sense to separate out the changes to `lib/Analysis/Loads.cpp`?
You should also make sure you can bootstrap clang with this change in place.
================
Comment at: include/llvm/Analysis/ValueTracking.h:231
@@ +230,3 @@
+
+ /// Returns true if this is always a dereferenceable pointer with alignment
+ /// greater or equal than requested. If the context instruction is specified
----------------
Nit: `this` should be `V`.
================
Comment at: lib/Analysis/Loads.cpp:68
@@ -67,1 +67,3 @@
const DataLayout &DL = ScanFrom->getModule()->getDataLayout();
+
+ // Require ABI alignment for loads without alignment specification
----------------
I'd put an assert here that `Align` is a power of 2.
================
Comment at: lib/Analysis/MemDerefPrinter.cpp:26
@@ -26,1 +25,3 @@
+ SmallVector<Value *, 4> Deref;
+ std::set<Value *> DerefAndAligned;
----------------
Use `DenseSet` or `SmallPtrSet`.
================
Comment at: lib/Analysis/MemDerefPrinter.cpp:72
@@ -67,1 +71,3 @@
V->print(OS);
+ if (DerefAndAligned.find(V) == DerefAndAligned.end())
+ OS << "\t(unaligned)";
----------------
Use `.count` (both `DenseSet` and `SmallPtrSet` have it).
================
Comment at: lib/Analysis/ValueTracking.cpp:2927
@@ +2926,3 @@
+ APInt BaseAlign(Offset.getBitWidth(), 0);
+ if (const AllocaInst *AI = dyn_cast<AllocaInst>(Base)) {
+ BaseAlign = AI->getAlignment();
----------------
The braces are unnecessary here.
================
Comment at: lib/Analysis/ValueTracking.cpp:3003
@@ -2976,3 +3002,3 @@
return false;
- if (!isDereferenceablePointer(Base, DL, CtxI,
- DT, TLI, Visited))
+ if (!isDereferenceableAndAlignedPointer(Base, Align, DL, CtxI, DT, TLI,
+ Visited))
----------------
Why do we have a restriction on `Base`s alignment here? We check below that `Base + Offset` is properly aligned, right? I'm not strongly opposed to keeping it like this, but please add a comment.
http://reviews.llvm.org/D9791
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list