Page MenuHomePhabricator

[LoopVersioning] Expose loop versioning as a pass too
ClosedPublic

Authored by anemet on Jan 26 2016, 4:26 PM.

Details

Summary

LoopVersioning is a transform utility that transform passes can use to
run-time disambiguate may-aliasing accesses. I'd like to also expose as
pass to allow it to be unit-tested.

I am planning to add support for non-aliasing annotation in
LoopVersioning and I'd like to be able to write tests directly using
this pass.

(After that feature is done, the pass could also be used to look for
optimization opportunities that are hidden behind incomplete alias
information at compile time.)

The pass drives LoopVersioning in its default way which is to fully
disambiguate may-aliasing accesses no matter how many checks are
required.

Diff Detail

Repository
rL LLVM

Event Timeline

anemet updated this revision to Diff 46075.Jan 26 2016, 4:26 PM
anemet retitled this revision from to [LoopVersioning] Expose loop versioning as a pass too.
anemet updated this object.
anemet added reviewers: hfinkel, ashutosh.nema, sbaranga.
anemet added a subscriber: llvm-commits.
zzheng added a subscriber: zzheng.Jan 27 2016, 12:17 PM
sbaranga accepted this revision.Feb 1 2016, 3:18 AM
sbaranga edited edge metadata.

Hi Adam,

Thanks, I can see how this can be very useful.
Are you planning on using this for anything other than unit testing?

Otherwise, LGTM!

Thanks,
Silviu

lib/Transforms/Utils/LoopVersioning.cpp
166 ↗(On Diff #46075)

nit:

vectorize -> version
distributing -> versioning

This revision is now accepted and ready to land.Feb 1 2016, 3:18 AM
sbaranga requested changes to this revision.Feb 1 2016, 3:54 AM
sbaranga edited edge metadata.

I have to retract my previous LGTM, I just spotted something that needs updating.

lib/Transforms/Utils/LoopVersioning.cpp
180 ↗(On Diff #46075)

This needs to be (LAI.getNumRuntimePointerChecks() || !LAI.PSE.getUnionPredicate().isAlwaysTrue()).

Maybe it would also make sense to have a separate method in LAI for this logic (something like needsRuntimeChecks()?)

This revision now requires changes to proceed.Feb 1 2016, 3:54 AM
anemet marked 2 inline comments as done.Feb 2 2016, 9:29 AM

Thanks for the review, Silviu.

Are you planning on using this for anything other than unit testing?

Not really other than for experimentation. I may propose a patch to schedule an off-by-default versioning early on in the pass pipeline. This could be tool for finding missed optimization due to incomplete alias information.

I also just wanted to mention the llvm-dev thread where this was discussed:

http://thread.gmane.org/gmane.comp.compilers.llvm.devel/86024

lib/Transforms/Utils/LoopVersioning.cpp
180 ↗(On Diff #46075)

Ah, of course, thanks for catching this!

I didn't add a separate method because looking at the clients, it suggest that the pattern of usage is more like:

  • check the number of memchecks, optionally by filtering the checks, fail if too many
  • check the number of SCEV predicate checks, fail if too many
  • if any was needed, version
anemet updated this revision to Diff 46667.Feb 2 2016, 9:33 AM
anemet edited edge metadata.
anemet marked an inline comment as done.

Address Silviu's comments

sbaranga accepted this revision.Feb 2 2016, 10:00 AM
sbaranga edited edge metadata.

Thanks, Adam! LGTM

-Silviu

This revision is now accepted and ready to land.Feb 2 2016, 10:00 AM
This revision was automatically updated to reflect the committed changes.