Modularizing LICM

Hi,

I’m writing a new loop pass, and has a need to call LICM(Loop Invariant Code Motion) ‘PromoteAliasSet’ on modified loop.

For now I didn’t found any why to call ‘PromoteAliasSet’ from my pass explicitly.

The only way is to schedule LICM pass after my pass.

For some reason my pass need more control and preferring to call LICM ‘PromoteAliasSet’ instead running full LICM as next pass.

Can LICM be more modular by exposing wrapper functions for its core functionalities (i.e. SinkRegion, HoistRegion, PromoteAliasSet) ?

These wrappers will check pre-requisite(i.e. AliasAnalysis, LoopInfo availability ) and call appropriate functions.

Regards,

Ashutosh

I’ve come across similar use cases recently. In particular, there are some cases where I’d like run HoistRegion on a particular basic block after loop unswitching.

I would be open to reviewing patches if you wanted to send them. I may get around to doing so myself eventually, but it’s fairly low on my priority list.

Philip

One way you could go is to expose the interface in
include/llvm/Transforms/Utils/LoopUtils.h. There's a similar approach
in the LCSSA and LoopSimplify passes, both define functions used by
other passes (e.g LoopUnroll and LICM).

Thanks Philip and Bruno.

I agree with Bruno’s suggestion, we can expose LICM core functionality(i.e. SinkRegion, HoistRegion, PromoteAliasSet) in LoopUtils.

Below are high level changes identified for it:

1) Expose SinkRegion, HoistRegion, PromoteAliasSet functions in LoopUtils.h
These functions will be accepting all the required analysis and pre-requisite.
i.e.: AliasAnalysis* , LoopInfo *, DominatorTree *, DataLayout *, TargetLibraryInfo *, Loop * , AliasSetTracker *

2) Implementation of SinkRegion, HoistRegion, PromoteAliasSet & supporting functions will be in a new file inside Utils.
(i.e.: /llvm/lib/Transforms/Utils/LICMCore.cpp)
These functions will first ensure the pre-requisite then only perform actions.
If any pre-requisite is un-available then the simply return.
LICM will no more have implementation for SinkRegion, HoistRegion, PromoteAliasSet and supporting functions.

3) LoopPromoter will be moved to Utils (i.e. LICMCore.cpp)

4) Who ever makes call to SinkRegion, HoistRegion, PromoteAliasSet has responsibility to pass required analysis and pre-requisites.

5) LICM pass will became more like a wrapper pass, calling SinkRegion, HoistRegion, PromoteAliasSet defined in Utils.

Suggestions are welcome.

Regards,
Ashutosh

Thanks Philip and Bruno.

I agree with Bruno’s suggestion, we can expose LICM core functionality(i.e. SinkRegion, HoistRegion, PromoteAliasSet) in LoopUtils.

Below are high level changes identified for it:

1) Expose SinkRegion, HoistRegion, PromoteAliasSet functions in LoopUtils.h
These functions will be accepting all the required analysis and pre-requisite.
i.e.: AliasAnalysis* , LoopInfo *, DominatorTree *, DataLayout *, TargetLibraryInfo *, Loop * , AliasSetTracker *

2) Implementation of SinkRegion, HoistRegion, PromoteAliasSet & supporting functions will be in a new file inside Utils.
(i.e.: /llvm/lib/Transforms/Utils/LICMCore.cpp)
These functions will first ensure the pre-requisite then only perform actions.
If any pre-requisite is un-available then the simply return.

To this last sentence, no. Failure to provide the required preconditions should be an assertion failure.

LICM will no more have implementation for SinkRegion, HoistRegion, PromoteAliasSet and supporting functions.

3) LoopPromoter will be moved to Utils (i.e. LICMCore.cpp)

4) Who ever makes call to SinkRegion, HoistRegion, PromoteAliasSet has responsibility to pass required analysis and pre-requisites.

5) LICM pass will became more like a wrapper pass, calling SinkRegion, HoistRegion, PromoteAliasSet defined in Utils.

Suggestions are welcome.

I think you're overall approach seems reasonable, but I'd strongly suggest starting with a single function (e.g. HoistRegion). There's a lot of details to work out in terms of interface. (For example, are we separating the 'known safe to speculate' and 'known to execute' cases? If not, how are we handling the in the MayThrow state?) Let's get agreement on a small change in the review thread, then take the agreement and apply it to more places.

To minimize the diff being discussed, I might even extract the helper function within the current file and move them to their new location as a separate change.