Page MenuHomePhabricator

[MemorySSA] Small fix for the clobber limit.
ClosedPublic

Authored by asbirlea on Apr 9 2019, 11:58 AM.

Details

Summary

After introducing the limit for clobber walking, walkToPhiOrClobber would assert that the limit is at least 1 on entry.
The test included triggered that assert.

The callsite in tryOptimizePhi making the calls to walkToPhiOrClobber is structured like this:

while (true) {
   if (getBlockingAccess()) { // calls walkToPhiOrClobber
   }
   for (...) {
     walkToPhiOrClobber();
   }
}

The cleanest fix is to check if the limit was reached inside walkToPhiOrClobber, and give an allowence of 1.
This approach not make any alias() calls (no calls to instructionClobbersQuery), so the performance condition is enforced.
The limit is set back to 0 if not used, as this provides info on the fact that we stopped before reaching a true clobber.

Diff Detail

Repository
rL LLVM

Event Timeline

asbirlea created this revision.Apr 9 2019, 11:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 9 2019, 11:58 AM
Herald added subscribers: Prazek, jlebar. · View Herald Transcript

LGTM after a nit is addressed. Thanks!

lib/Analysis/MemorySSA.cpp
553 ↗(On Diff #194375)

For clarity's sake, can we please explicitly = 1 here (and = 0 in the new conditional below), rather than incrementing/decrementing?

This revision is now accepted and ready to land.Apr 11 2019, 1:59 PM
This revision was automatically updated to reflect the committed changes.