Page MenuHomePhabricator

[LICM] Break the assumption that the sub loops cannot be modified by other loop passes
AbandonedPublic

Authored by haicheng on Feb 17 2016, 10:34 PM.

Details

Reviewers
mssimpso
mcrosier
Summary

LICM assumes that the sub loops cannot be modified by other loop passes (See the deleted comment starting with "What if"). It is true for the current loop optimization pipeline. However, if the pipeline is changed, the assumption may not be true (e.g. test/Transforms/LICM/AliasSetMemSet.ll which is discussed in this thread).

This patch first tests if there are more than one loop passes run together. If it is true, it calculates the AST from the scratch without using the LoopToAliasSetMap cache. This patch has no impact on either function or compilation time for the current loop optimization pipeline.

Diff Detail

Repository
rL LLVM

Event Timeline

haicheng updated this revision to Diff 48279.Feb 17 2016, 10:34 PM
haicheng retitled this revision from to [LICM] Break the assumption that the sub loops cannot be modified by other loop passes.
haicheng updated this object.
haicheng added reviewers: mssimpso, mcrosier, gberry.
haicheng set the repository for this revision to rL LLVM.
haicheng added a subscriber: llvm-commits.
mssimpso edited edge metadata.Feb 18 2016, 12:16 PM

Haicheng, would it be difficult construct a test case here that demonstrates the caching problem? Perhaps something like the memset example that exposed the issue?

Haicheng, would it be difficult construct a test case here that demonstrates the caching problem? Perhaps something like the memset example that exposed the issue?

I think this test case (test/Transforms/LICM/AliasSetMemSet.ll) can show the problem. Do you think it is good enough?

gberry resigned from this revision.May 23 2016, 3:26 PM
gberry removed a reviewer: gberry.
haicheng abandoned this revision.May 25 2016, 11:30 AM

Now LICM can update AST if the sub-loops are changed by loop unroll. It might be better to do the same for LIR instead of re-computing the AST.