[PATCH] Take alignment into account in isSafeToLoadUnconditionally

Philip Reames listmail at philipreames.com
Tue Jun 16 10:14:01 PDT 2015


================
Comment at: lib/Analysis/Loads.cpp:69
@@ +68,3 @@
+
+  // Require ABI alignment for loads without alignment specification
+  if (Align == 0)
----------------
Please clarify comment as to *why* we want this.  

================
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)) {
----------------
>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.

================
Comment at: lib/Analysis/Loads.cpp:150
@@ -136,1 +149,3 @@
+      AccessedAlign = DL.getABITypeAlignment(AccessedTy);
+    if (AccessedAlign < Align)
       continue;
----------------
Why should the native alignment of the pointee type influence anything?  We can freely bitcast pointers before loading.

http://reviews.llvm.org/D10475

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






More information about the llvm-commits mailing list