[PATCH] Take alignment into account in isSafeToLoadUnconditionally

[Artur Pilipenko]

================
Comment at: lib/Analysis/Loads.cpp:137
@@ -130,2 +136,3 @@
     Value *AccessedPtr;
-    if (LoadInst *LI = dyn_cast<LoadInst>(BBI))
+    unsigned AccessedAlign;
+    if (LoadInst *LI = dyn_cast<LoadInst>(BBI)) {
----------------
reames wrote:
> From here on looks like a separate change.  Please separate.  Each logical change should be reviewed independently.  Also, you should have a test case for each.
Logically it's a single change to take alignment into account in this function. Actually from here on the main part of the change begins. I see no reason to split it more. Test case is also provided.  

================
Comment at: lib/Analysis/Loads.cpp:150
@@ -136,1 +149,3 @@
+      AccessedAlign = DL.getABITypeAlignment(AccessedTy);
+    if (AccessedAlign < Align)
       continue;
----------------
reames wrote:
> Why should the native alignment of the pointee type influence anything?  We can freely bitcast pointers before loading.  
If we end up here with zero AccessedAlign it means that there is load or store instruction before without explicit alignment specification. From docs about load/stores:
"A value of 0 or an omitted align argument means that the operation has the ABI alignment for the target." 
"Overestimating the alignment results in undefined behavior."
So, I assume here that if previous load succeed that pointer alignment is at least ABI alignment.

http://reviews.llvm.org/D10475

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