[PATCH] D109354: [LoopBoundSplit] Check the start value of split cond AddRec

JinGu Kang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 13 02:11:30 PDT 2021


jaykang10 added a comment.

In D109354#2996639 <https://reviews.llvm.org/D109354#2996639>, @mkazantsev wrote:

> LGTM, thanks!

Thanks for review @mkazantsev! Let me push this patch and create a new patch following your comments.



================
Comment at: llvm/lib/Transforms/Scalar/LoopBoundSplit.cpp:272
+    const SCEV *SplitAddRecStartSCEV =
+        cast<SCEVAddRecExpr>(SplitCandidateCond.AddRecSCEV)->getStart();
+    if (!SE.isKnownPredicate(SplitCandidateCond.Pred, SplitAddRecStartSCEV,
----------------
mkazantsev wrote:
> As a follow-up (no need to do it in this patch): can AddRecSCEV have the type of `SCEVAddRecExpr` so that this cast is not needed?
Yep, let me update it in new patch.


================
Comment at: llvm/lib/Transforms/Scalar/LoopBoundSplit.cpp:273
+    const SCEV *SplitAddRecStartSCEV =
+        dyn_cast<SCEVAddRecExpr>(SplitCandidateCond.AddRecSCEV)->getStart();
+    if (SE.isKnownPredicate(
----------------
mkazantsev wrote:
> jaykang10 wrote:
> > mkazantsev wrote:
> > > This should either be `cast` instead of `dyn_cast` (if it never fails), or there should be a null check (if it may fail).
> > Sorry, let me update it with cast.
> As a follow-up: consider using `isLoopEntryGuardedByCond`. Maybe the 1st iter condition can be inferred from a guarding check. Can be a separate patch.
Yep, let me try to use the `isLoopEntryGuardedByCond` in new patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109354/new/

https://reviews.llvm.org/D109354



More information about the llvm-commits mailing list