Page MenuHomePhabricator

[IndVarSimplify][NFC] Removed mayThrow from if-condition in predicateLoopExits of IndVarSimplify
ClosedPublic

Authored by vdsered on May 1 2021, 12:21 AM.

Details

Summary

Instruction has mayHaveSideEffects method that returns true if mayThrow return true because this is called internally in the first method

Diff Detail

Event Timeline

vdsered created this revision.May 1 2021, 12:21 AM
vdsered requested review of this revision.May 1 2021, 12:21 AM
vdsered retitled this revision from Removed mayThrow from if-condition in predicateLoopExits of IndVarSimplify to [IndVarSimplify][NFC] Removed mayThrow from if-condition in predicateLoopExits of IndVarSimplify.May 1 2021, 12:23 AM
xbolva00 added inline comments.
llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
1673

Or better, implement TODO?

xbolva00 added inline comments.
llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
1672

Maybe we should have some scanlimit for huge BBs?

nikic added inline comments.May 1 2021, 4:38 AM
llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
1673

Ideally we should be using SCEV loop properties here, which are cached. Unfortunately, the SCEV notion of "side effect" differs from what is needed here (SCEV ignores simple writes).

We could at least use loopHasNoAbnormalExits() though, which uses isGuaranteedToTransfer(). And I do believe that one is needed here -- I don't think the transform is valid if the loop contains a non-willreturn readonly call. It would be good to add a test for that case.

reames accepted this revision.May 1 2021, 11:38 AM

LGTM.

(I'm ignoring the side discussion as it doesn't appear to relate to the patch which is simply a small cleanup.)

This revision is now accepted and ready to land.May 1 2021, 11:38 AM

@reames, I don't have commit access. Can you commit the patch?