This is an archive of the discontinued LLVM Phabricator instance.

[LICM] Report successful hoists/sinks
ClosedPublic

Authored by anemet on Dec 19 2016, 1:34 PM.

Event Timeline

anemet updated this revision to Diff 81992.Dec 19 2016, 1:34 PM
anemet retitled this revision from to [LICM] Report successful hoists/sinks.
anemet updated this object.
anemet added a reviewer: hfinkel.
anemet added a subscriber: llvm-commits.
sanjoy added a comment.Jan 3 2017, 9:46 AM

How about adding remarks to promoteLoopAccessesToScalars? Or did you intend to do that later?

davide requested changes to this revision.Jan 3 2017, 9:50 AM
davide added a reviewer: davide.
davide added a subscriber: davide.

I think this needs more tests (a suggestion inline).

test/Transforms/LICM/opt-remarks.ll
11–16

This test is fine but you should add another one where you show up that the remark is emitted when a load is sunk

This revision now requires changes to proceed.Jan 3 2017, 9:50 AM
anemet added inline comments.Jan 3 2017, 10:04 AM
test/Transforms/LICM/opt-remarks.ll
11–16

You mean, store, right?

davide added inline comments.Jan 3 2017, 10:05 AM
test/Transforms/LICM/opt-remarks.ll
11–16

yeah, fat fingering =)

How about adding remarks to promoteLoopAccessesToScalars? Or did you intend to do that later?

Sure, I can add them to that too.

anemet updated this revision to Diff 83101.Jan 4 2017, 11:51 AM
anemet edited edge metadata.

Address both Sanjoy's and Davide's comments.

davide accepted this revision.Jan 6 2017, 6:29 PM
davide edited edge metadata.

LGTM

lib/Transforms/Scalar/LICM.cpp
220–226

runOnLoop and friends now take 8 arguments. I think it's a bit too much. Not your fault but clearly a sign that LICM structure could use some love.

This revision is now accepted and ready to land.Jan 6 2017, 6:29 PM
davide added inline comments.Jan 6 2017, 6:31 PM
include/llvm/Transforms/Utils/LoopUtils.h
407–412

This comment needs to be updated to reflect the fact we now also take an ORE

417–423

Ditto.

428–437

Ditto.

This revision was automatically updated to reflect the committed changes.