[PATCH] D102234: [SimpleLoopBoundSplit] Split Bound of Loop which has conditional branch with IV

JinGu Kang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 17 03:23:56 PDT 2021


jaykang10 added a comment.

In D102234#2762711 <https://reviews.llvm.org/D102234#2762711>, @SjoerdMeijer wrote:

> Some drive by comments:

@SjoerdMeijer Thanks for comments.

> - Bike shedding names:  `SimpleLoopBoundSplit`. How about just `LoopBoundSplit`? I understand the current implementation has restrictions/limitations, but personally I think it looks a bit silly all these SimpleLoopXYZ passes/names.

Yep, I am ok with the name. I just imitated the `LoopUnswitch` and `SimpleLoopUnswitchPass` because this pass is a limited version of IRCE.

> - You probably want to add this to the optimisation pipeline somewhere, off by default for now,

Yep, I have enabled this pass after SimpleLoopUnswitch experimentally and I am checking the impact from benchmarks. It seems the `isProfitableToTransform` needs to be updated.

> - I guess we want to skip with this transformation with OptForSize,

Yep, it is already being checked with below code.

  // Skip function with optsize.
  if (L.getHeader()->getParent()->hasOptSize())
    return false;



> - I haven't looked into much details, but I guess you look only at one if-statement and thus we don't support things like `if ( i > c1 && i < c2)`?

At this stage, I am aiming this pass as simple as possible. As @reames mentioned on the email discussion, it is very hard to generalize the cases. Later, we could create new transformation passes or extend this pass to handle more cases as @mkazantsev mentioned.


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

https://reviews.llvm.org/D102234



More information about the llvm-commits mailing list