Page MenuHomePhabricator

Convert atomicrmws to xchg or store where legal
ClosedPublic

Authored by reames on Feb 15 2019, 9:41 AM.

Details

Summary

Implement two more transforms of atomicrmw in InstCombine.

  1. We can convert an atomicrmw which produces a known value in memory into an xchg instead.
  2. We can convert an atomicrmw xchg w/o users into a store for some orderings.

Diff Detail

Repository
rL LLVM

Event Timeline

reames created this revision.Feb 15 2019, 9:41 AM
jfb accepted this revision.Feb 15 2019, 9:55 AM

A few nits, LGTM otherwise.

lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
52 ↗(On Diff #187031)

"its value's"

59 ↗(On Diff #187031)

You can move RMWI.getOperation() in here.

73 ↗(On Diff #187031)

FAdd / FSub with any NaN are also saturating.

Technically xchg is also saturating, pre your description above...

99 ↗(On Diff #187031)

Do you want to factor this out and run it after isSaturating succeeds above? That way instcombine doesn't have to do two passes to do this transform.

133 ↗(On Diff #187031)

I guess here you could have done return eraseInstFromFunction(RMWI); as well?

This revision is now accepted and ready to land.Feb 15 2019, 9:55 AM
reames marked 3 inline comments as done.Feb 15 2019, 1:14 PM
reames added inline comments.
lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
59 ↗(On Diff #187031)

Will do this and the other site of the same in a follow up nfc

99 ↗(On Diff #187031)

Saving a cycle through the iteration didn't seem worth code complexity.

133 ↗(On Diff #187031)

InstCombine does that automatically for non-void instructions.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 15 2019, 1:30 PM