Page MenuHomePhabricator

[SimpleLoopUnswitch] Early check exit for trivial unswitch with MemorySSA.
ClosedPublic

Authored by asbirlea on Jan 23 2019, 5:15 PM.

Details

Summary

If MemorySSA is avaiable, we can skip checking all instructions if block has any Defs.
(volatile loads are also Defs).
We still need to check all instructions for "canThrow", even if no Defs are found.

Diff Detail

Repository
rL LLVM

Event Timeline

asbirlea created this revision.Jan 23 2019, 5:15 PM
lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
852 ↗(On Diff #183217)

Do atomic ops fall in the same "we care and should exit now" bucket as volatile loads for this?

MSSA uses MemoryDefs to represent fences and atomic loads with ordering > unordered.

chandlerc accepted this revision.Jan 25 2019, 7:42 PM

LGTM (provided I've not missed anything below w.r.t. atomics).

lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
852 ↗(On Diff #183217)

I think so yes, and so I think this code is correct?

Such loads "appear" to write to memory and thus I think should bail. At least, this code seems to be remain a good early exit. We still directly test every instruction below.

This revision is now accepted and ready to land.Jan 25 2019, 7:42 PM
asbirlea marked 3 inline comments as done.Jan 28 2019, 9:47 AM

Thank you for the review!

lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
852 ↗(On Diff #183217)

The check below is mayHaveSideEffects, which internally calls mayWriteToMemory, which returns true for fences and unordered loads.
So, yes, we were already bailing for these.

This revision was automatically updated to reflect the committed changes.
asbirlea marked an inline comment as done.