This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Improve codegen for i8/i16 'atomicrmw xchg a, {0,-1}'
ClosedPublic

Authored by asb on Aug 1 2023, 7:54 AM.

Details

Summary

As noted in https://github.com/llvm/llvm-project/issues/64090, it's more efficient to lower a partword 'atomicrmw xchg a, 0` to and amoand with appropriate mask. There are a range of possible ways to go about this - e.g. writing a combine based on the llvm.riscv.masked.atomicrmw.xchg intrinsic, or introducing a new interface to AtomicExpandPass to allow target-specific atomics conversions, or trying to lift the conversion into AtomicExpandPass itself based on querying some target hook. Ultimately I've gone with what appears to be the simplest approach - just covering this case in emitMaskedAtomicRMWIntrinsic. I perhaps should have given that hook a different name way back when it was introduced.

This also handles the atomicrmw xchg a, -1 case suggested by Craig during review.

Fixes https://github.com/llvm/llvm-project/issues/64090

Diff Detail

Event Timeline

asb created this revision.Aug 1 2023, 7:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2023, 7:54 AM
asb requested review of this revision.Aug 1 2023, 7:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2023, 7:54 AM
craig.topper added inline comments.Aug 1 2023, 8:43 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
16750

We can also support -1 using amoor.

asb updated this revision to Diff 546132.Aug 1 2023, 10:44 AM
asb retitled this revision from [RISCV] Improve codegen for i8/i16 'atomicrmw xchg a, 0' to [RISCV] Improve codegen for i8/i16 'atomicrmw xchg a, {0,-1}' .
asb edited the summary of this revision. (Show Details)

Update to cover the atomicrmw xchg a, -1 case as well.

asb updated this revision to Diff 546134.Aug 1 2023, 10:45 AM
craig.topper accepted this revision.Aug 1 2023, 2:09 PM

LGTM with that comment addressed.

I had tried to do this as part of in widenPartwordAtomicRMW in AtomicExpandPass.cpp. That regressed in VE which has a pretty capable xchg instruction with builtin masking but not and/or. After that failed attempt I was going to write basically this patch, but you beat me to it. Thanks.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
16756

static_cast -> cast. I know you already did isa but cast is the llvm way.

This revision is now accepted and ready to land.Aug 1 2023, 2:09 PM
asb updated this revision to Diff 546357.Aug 2 2023, 1:48 AM
asb marked 2 inline comments as done.
This revision was landed with ongoing or failed builds.Aug 2 2023, 1:50 AM
This revision was automatically updated to reflect the committed changes.