[PATCH] D33129: [SCEVExpander] Try harder to avoid introducing inttoptr

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy requested changes to this revision.
sanjoy added a comment.
This revision now requires changes to proceed.

Thank you for doing this!



================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:1307
   // expand to an integer type to avoid the need for additional casting.
   Type *ExpandTy = PostLoopScale ? IntTy : STy;
+  // We can't use a pointer type for the addrec if the pointer type is
----------------
Did you consider just making this `Type *ExpandTy = PostLoopScale ? IntTy : Normalized->getType();`?


================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:1392
+      if (Result->getType()->isIntegerTy()) {
+        Value *Base = expandCodeFor(PostLoopOffset, ExpandTy);
+        const SCEV *const OffsetArray[1] = {SE.getUnknown(Result)};
----------------
This is to prevent an `ptrtoint` on the expansion of `PostLoopOffset`, right?  If so, I think phrasing this in terms of the type of `PostLoopOffset` may be more obvious.


================
Comment at: test/Transforms/LoopStrengthReduce/nonintegral.ll:19
+L86:                                              ; preds = %L86, %top
+  %i.0 = phi i64 [ 0, %top ], [ %0, %L86 ]
+  %0 = add i64 %i.0, 1
----------------
Run `-metarenamer` on `-instnamer` on this?

Also, a C++ unit test that directly uses SCEVExpander will be more robust.  This test case can become a no-op if LSR heuristics change.


https://reviews.llvm.org/D33129