This is an archive of the discontinued LLVM Phabricator instance.

[LoopInterchange] Do not interchange loops with function calls.
ClosedPublic

Authored by fhahn on Jul 17 2017, 8:50 AM.

Details

Summary

Without any information about the called function, we cannot be sure
that it is safe to interchange loops which contain function calls. For
example there could be dependences that prevent interchanging between
accesses in the called function and the loops. Even functions without any
parameters could cause problems, as they could access memory using
global pointers.

For now, I think it is only safe to interchange loops with calls marked
as readnone.

With this patch, the LLVM test suite passes with `-O3 -mllvm
-enable-loopinterchange` and LoopInterchangeProfitability::isProfitable
returning true for all loops. check-llvm and check-clang also pass when
bootstrapped in a similar fashion, although only 3 loops got
interchanged.

Diff Detail

Event Timeline

fhahn created this revision.Jul 17 2017, 8:50 AM
fhahn updated this revision to Diff 107699.Jul 21 2017, 12:47 PM
fhahn retitled this revision from [LoopInterchange] Do not interchange loops with calls to any functions. to [LoopInterchange] Do not interchange loops with calls to functions..
fhahn edited the summary of this revision. (Show Details)
fhahn added reviewers: hfinkel, mcrosier, mkuper.
mcrosier accepted this revision.Jul 28 2017, 7:33 AM

Sounds reasonable to me.

This revision is now accepted and ready to land.Jul 28 2017, 7:33 AM
mcrosier added inline comments.Jul 28 2017, 7:34 AM
test/Transforms/LoopInterchange/call-instructions.ll
60 ↗(On Diff #107699)

It might be worth while to future proof this by adding CHECK directives that actually make sure the CFG structure doesn't change.

fhahn updated this revision to Diff 108879.Jul 31 2017, 1:44 AM
fhahn retitled this revision from [LoopInterchange] Do not interchange loops with calls to functions. to [LoopInterchange] Do not interchange loops with function calls..

Added more checks to test case, thanks @mcrosier for having a look! :)

fhahn marked an inline comment as done.Jul 31 2017, 1:46 AM
fhahn closed this revision.Jul 31 2017, 2:01 AM