Page MenuHomePhabricator

[LowerAtomic] Lower fadd and fsub atomicrmw instructions
ClosedPublic

Authored by jberdine on Apr 15 2019, 3:03 AM.

Details

Reviewers
arsenm
reames
Summary

FAdd and FSub have recently (r351850) been added as atomicrmw
operations. This diff adds lowering cases for them to the LowerAtomic
transform.

Diff Detail

Event Timeline

jberdine created this revision.Apr 15 2019, 3:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 15 2019, 3:03 AM

Needs tests. Also I'm about 90% sure I added this already?

Apparently we have both lib/CodeGEn/AtomicExpandPass.cpp, and lib/Transforms/Scalar/LowerAtomic.cpp. It's unclear to me what the difference is, or why both exist

My understanding (which shouldn't be taken to imply much) is that lib/Transforms/Scalar/LowerAtomic.cpp is for contexts where atomic semantics is known to be unnecessary due to e.g. executing only in a sequential context.

This still needs tests, but it seems this duplicate pass thing could use some work. Both passes have essentially the same switch to turn the atomic into the equivalent non-atomic. It seems to be me like AtomicExpandPass is a superset of LowerAtomic, except AtomicExpand seems to ignore fences

jberdine updated this revision to Diff 195132.Apr 15 2019, 4:32 AM

add tests

I see what you mean about the redundancy between the LowerAtomic and AtomicExpand passes.

Are the added tests what you had in mind?

arsenm accepted this revision.Apr 17 2019, 5:00 AM

LGTM

This revision is now accepted and ready to land.Apr 17 2019, 5:00 AM

Thanks! Note that I don't have write access, so could I ask you to commit this?

compnerd closed this revision.May 23 2019, 10:01 AM
compnerd added a subscriber: compnerd.

SVN r361512