This is an archive of the discontinued LLVM Phabricator instance.

[LoongArch] Add codegen support for atomic fence, atomic load and atomic store
ClosedPublic

Authored by gonglingqin on Jun 30 2022, 2:58 AM.

Diff Detail

Event Timeline

gonglingqin created this revision.Jun 30 2022, 2:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 2:58 AM
gonglingqin requested review of this revision.Jun 30 2022, 2:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 2:58 AM
xry111 added inline comments.Jul 2 2022, 4:28 AM
llvm/test/CodeGen/LoongArch/ir-instruction/load-store-atomic.ll
139

GCC uses one instruction for this: amswap_db.d $zero, $a1, $a0, and to me it's correct. Can we also use it?

Likewise for i32 store release operation.

lkail added a subscriber: lkail.Jul 2 2022, 4:35 AM
lkail added inline comments.
llvm/lib/Target/LoongArch/LoongArchTargetMachine.cpp
116

You may need to add tests in lvm/test/Transforms/AtomicExpand/LoongArch.

gonglingqin added inline comments.Jul 3 2022, 8:35 PM
llvm/lib/Target/LoongArch/LoongArchTargetMachine.cpp
116

Thanks for reminding me.I will add tests.

llvm/test/CodeGen/LoongArch/ir-instruction/load-store-atomic.ll
139

Thank you for your reminder. This may be possible. Further, we can generate different instructions according to constraints (such as instructions without fences when unordered). I'm not sure whether this optimization should be implemented in this patch or another patch. Do you have any suggestions?

Address @lkail's comments.

SixWeining added inline comments.Jul 4 2022, 12:21 AM
llvm/test/Transforms/AtomicExpand/LoongArch/load-store-atomic.ll
2 ↗(On Diff #442001)

It should be --.

2 ↗(On Diff #442001)

ditto

xry111 added inline comments.Jul 4 2022, 12:29 AM
llvm/test/CodeGen/LoongArch/ir-instruction/load-store-atomic.ll
139

Maybe in another patch, as these two instructions are not explicitly emitted by the target code, but by some "internal LLVM magic".

gonglingqin added inline comments.Jul 4 2022, 1:10 AM
llvm/test/CodeGen/LoongArch/ir-instruction/load-store-atomic.ll
139

Thanks for the suggestion, I will handle it in another patch.

llvm/test/Transforms/AtomicExpand/LoongArch/load-store-atomic.ll
2 ↗(On Diff #442001)

Thanks. I will change that.

2 ↗(On Diff #442001)

Thanks. I will change that.

Address @SixWeining's comments.

xen0n accepted this revision.Jul 11 2022, 12:00 AM

I agree with @xry111 that the optimization to use amswap_db.d $zero, x, y for atomic stores can be done in a later patch. For now this LGTM but I'd like someone else to take a look too.

This revision is now accepted and ready to land.Jul 11 2022, 12:00 AM
SixWeining accepted this revision.Jul 13 2022, 12:05 AM
This revision was landed with ongoing or failed builds.Jul 13 2022, 12:27 AM
This revision was automatically updated to reflect the committed changes.