[PATCH] D41953: [LoopUnroll] Unroll and Jam

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D41953#1113514, @echristo wrote:

> It sounded like Hal wanted to review this


Thanks Eric. I don't need to review this if others who are qualified look at it. I've skimmed the code and it seems reasonable. There are a number of obvious enhancements that might be worthwhile, but I think it's best to work on those after this lands.

> and I don't know that any of the other people on the review line have any experience with loop passes and so probably shouldn't have been approving this. I might be wrong here, and if so I apologize, but it seems like this went in a bit early.
> 
> From a quick glance it also looks like the testcases could be cleaned up a bit. In particular the naming if nothing else.
> 
> Thanks.





================
Comment at: llvm/trunk/lib/Transforms/Utils/LoopUnrollAndJam.cpp:726
+  const SCEV *TripCountSC =
+      SE.getAddExpr(BECountSC, SE.getConstant(BECountSC->getType(), 1));
+  if (isa<SCEVCouldNotCompute>(TripCountSC))
----------------
Can this overflow if the exit count is INT_MAX (or similar)? It's generally a good idea to comment on how that's handled.


Repository:
  rL LLVM

https://reviews.llvm.org/D41953