Page MenuHomePhabricator

[AtomicExpandPass]: Add a hook for custom cmpxchg expansion in IR
ClosedPublic

Authored by asb on Jun 13 2018, 8:18 AM.

Details

Summary

This involves changing the shouldExpandAtomicCmpXchgInIR interface, but I have updated the in-tree backends using this hook (ARM, AArch64, Hexagon) so they will see no functional change.

Posting to allow discussion of the full patch series in my associated RFC.

Diff Detail

Repository
rL LLVM

Event Timeline

asb created this revision.Jun 13 2018, 8:18 AM
asb edited the summary of this revision. (Show Details)Jun 13 2018, 8:47 AM

It seems a bit of a shame to hide that masked expansion away in lib/Target/RISCV when MIPS (at least) needs similar logic. I suppose someone who wants to refactor MIPS can move it out again though.

include/llvm/CodeGen/TargetLowering.h
1533–1534 ↗(On Diff #151174)

The return value is never used, so it might be an idea to remove it. If not, I'm not really convinced by the comment: "appropriately shifted/masked oldval" is both not what actually gets returned and overly specific to RISC-V's reasons for doing it in custom code.

asb updated this revision to Diff 165970.Sep 18 2018, 7:17 AM
asb marked an inline comment as done.
asb edited the summary of this revision. (Show Details)

I've changed this to better match the revised approach taken for atomicrmw expansion, which maximises the amount of code that can be shared between targets. See D48131 to see this interface put to use.

asb added a comment.Sep 18 2018, 7:17 AM

It seems a bit of a shame to hide that masked expansion away in lib/Target/RISCV when MIPS (at least) needs similar logic. I suppose someone who wants to refactor MIPS can move it out again though.

I agree. I've restructured things so that as much masked expansion as possible is in AtomicExpandPass and can be reused across targets.

jyknight accepted this revision.Sep 18 2018, 9:11 AM

Straightforward enough; LG.

This revision is now accepted and ready to land.Sep 18 2018, 9:11 AM
This revision was automatically updated to reflect the committed changes.