[PATCH] D14599: [safestack] Rewrite isAllocaSafe using SCEV.

[Peter Collingbourne via llvm-commits]

pcc accepted this revision.
pcc added a comment.
This revision is now accepted and ready to land.

LGTM


================
Comment at: lib/Transforms/Instrumentation/SafeStack.cpp:186
@@ +185,3 @@
+  ConstantRange AccessStartRange = SE->getUnsignedRange(Expr);
+  ConstantRange SizeRange =
+      ConstantRange(APInt(BitWidth, 0), APInt(BitWidth, Size));
----------------
eugenis wrote:
> Is it because we are comparing it with a non-negative alloca range, and even if the signed range is "smaller", it would still overflow in the negative direction?
> 
> Switched to getUnsignedRange.
> 
> 
I agree that you just need `getUnsignedRange`. My understanding is that the two functions are supposed to return essentially the same thing but return slightly different results in the negative range for unknowns (e.g. for unknowns with m trailing zeros [0,2^n-2^m) vs [-2^(n-1)..2^(n-1)-2^m) ) which we don't care about here.

================
Comment at: lib/Transforms/Instrumentation/SafeStack.cpp:301
@@ +300,3 @@
+      default:
+        if (Visited.insert(I).second)
+          WorkList.push_back(cast<const Instruction>(I));
----------------
After sleeping on it I agree that your approach is correct.


Repository:
  rL LLVM

http://reviews.llvm.org/D14599