[PATCH] [SLSR] handle candidate form &B[i * S]

Sanjoy Das sanjoy at playingwithpointers.com
Mon Mar 9 16:16:08 PDT 2015


Few drop-by comments inline.


================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:93
@@ +92,3 @@
+          Basis(nullptr) {}
+    Type CandidateType;
+    const SCEV *Base;
----------------
Very minor:  I'd call this `Kind` to differentiate from `llvm::Type`.

================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:327
@@ +326,3 @@
+  if (MaxValue.getZExtValue() >= ElementSize) {
+    // I = B + sext((Idx * ElementSize) *s S)
+    ConstantInt *ScaledIdx = ConstantInt::get(
----------------
Unless I'm missing some larger context; for this equivalence to hold, you'll have to prove that `Idx * S * ElementSize` does not sign-overflow.  For instance, consider:

`Idx->getType()` == `i8`, pointers are 16 bit, `Idx` == `4`, `S` == `4` and `ElementSize` == `24`.  `sext(Idx *s S) *s ElementSize` is `i16 384` while `sext((Idx * ElementSize) *s S)` is `-128`.

================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:444
@@ +443,3 @@
+        // C = inttoptr(ptrtoint(Basis) + Bump)
+        Reduced = Builder.CreatePtrToInt(Basis.Ins, IntPtrTy);
+        Reduced = Builder.CreateAdd(Reduced, Bump, "", false, true);
----------------
Have you considered doing a `bitcast` to `i8*`, gep on that `i8*` and then back instead?  That will preserve pointer-ness throughout.

================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:451
@@ +450,3 @@
+  default:
+    assert(false && "CandidateType is invalid");
+  };
----------------
Maybe use `llvm_unreachable` here?

http://reviews.llvm.org/D7459

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






More information about the llvm-commits mailing list