Page MenuHomePhabricator

Move the check of EnablePostRAScheduler to avoid side effect of disabling antidependency breaker
ClosedPublic

Authored by mbodart on May 11 2016, 4:01 PM.

Details

Summary

With the current code structure, specifying -post-RA-scheduler=true has different behavior than
enabling it via the scheduling model. The issue is that the mere presence of this option on the
command line suppresses the post scheduler code that would otherwise select an antidependency breaker.

I discovered this when chasing http://llvm.org/PR27681, which should have been reproducible
with -post-RA-scheduler=true. With this change, that will be the case, and should help avoid
such surprises down the road.

I added the "private:" specification simply to make it a little more clear that PostRAScheduler::enablePostRAScheduler
is only referenced from within its class.

Diff Detail

Repository
rL LLVM

Event Timeline

mbodart updated this revision to Diff 56975.May 11 2016, 4:01 PM
mbodart retitled this revision from to Move the check of EnablePostRAScheduler to avoid side effect of disabling antidependency breaker.
mbodart updated this object.
mbodart added a reviewer: spatel.
mbodart added a subscriber: llvm-commits.
mgrang added a subscriber: mgrang.May 11 2016, 4:07 PM
mgrang added inline comments.
lib/CodeGen/PostRASchedulerList.cpp
296 ↗(On Diff #56975)

You can get rid of the extra newline.

spatel accepted this revision.May 12 2016, 6:59 AM
spatel edited edge metadata.

LGTM.

This revision is now accepted and ready to land.May 12 2016, 6:59 AM
mbodart added inline comments.May 16 2016, 8:54 AM
lib/CodeGen/PostRASchedulerList.cpp
296 ↗(On Diff #56975)

Can you clarify which line is "extra"?

There's one line of white space before, separating the code from some local declarations,
and one after. Both lead to better readability, IMO.

This revision was automatically updated to reflect the committed changes.