Page MenuHomePhabricator

[NFC][LICM] Remove too conservative IsMustExecute variable
ClosedPublic

Authored by mkazantsev on Aug 17 2018, 3:14 AM.

Details

Summary

LICM relies on variable MustExecute which is conservatively set to false in all non-headers.
It is used when we decide whether or not we want to hoist an instruction or a guard.

For instructions, the check in isSafeToExecuteUnconditionally is strictly more precise than this
variable, so we can safely give it up.

For the guards, it might be too conservative to use this variable, we can instead use a more precise
logic from MustExecute. Currently it is only NFC because IsMemoryNotModified is also conservatively
set to false for all non-headers, and we cannot hoist guards from non-header blocks. However once
we give up using IsMemoryNotModified and use a smarter check instead, this will allow us to hoist
guards from all mustexecute non-header blocks.

Diff Detail

Repository
rL LLVM

Event Timeline

fedor.sergeev added inline comments.Aug 17 2018, 6:04 AM
lib/Transforms/Scalar/LICM.cpp
464 ↗(On Diff #161194)

Since you are going to remove isGuaranteedToExecute perhaps mention something else here?
Or just drop the reference.

mkazantsev added inline comments.Aug 20 2018, 12:58 AM
lib/Transforms/Scalar/LICM.cpp
464 ↗(On Diff #161194)

Right, I will do it when I drop it.

mkazantsev retitled this revision from [NFC][LICM] Remove too conservative variable to [NFC][LICM] Remove too conservative IsMustExecute variable.Aug 21 2018, 9:07 PM
This revision is now accepted and ready to land.Aug 22 2018, 1:39 AM
reames added inline comments.Aug 22 2018, 12:28 PM
lib/Transforms/Scalar/LICM.cpp
523 ↗(On Diff #161194)

We should be able to sink this inside the normal hoisting now since the must execute case is now handled. Suggestion as a follow on change, nothing more.

This revision was automatically updated to reflect the committed changes.