This is an archive of the discontinued LLVM Phabricator instance.

[LoongArch] Add atomic ordering information for binary atomic operations
ClosedPublic

Authored by gonglingqin on Nov 22 2022, 1:24 AM.

Details

Summary

This patch also implements not emit fence in atomic binary operation
when AtomicOrdering is monotonic and fixes the issue of loading from
non ptr parameters.

The processing of other levels of AtomicOrdering will be added later.

Diff Detail

Event Timeline

gonglingqin created this revision.Nov 22 2022, 1:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 22 2022, 1:24 AM
gonglingqin requested review of this revision.Nov 22 2022, 1:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 22 2022, 1:24 AM

Mostly LGTM, I was considering this optimization just before seeing this diff.

But why will this change affect register allocation?

xry111 added inline comments.Nov 22 2022, 1:41 AM
llvm/test/CodeGen/LoongArch/ir-instruction/atomicrmw.ll
685–687

I mean, why this is changed?

gonglingqin added inline comments.Nov 22 2022, 2:16 AM
llvm/test/CodeGen/LoongArch/ir-instruction/atomicrmw.ll
685–687

Thanks for your review. This change is due to previously generated assembly incorrectly loading from non-ptr parameters. This patch fixes this problem. Do you have any suggestions as to whether I should add a description of this information in the summary or split the bugfix into a separate patch?

xry111 added inline comments.Nov 22 2022, 2:29 AM
llvm/test/CodeGen/LoongArch/ir-instruction/atomicrmw.ll
685–687

Ah, get it.

It's generally better to separate "bugfix" and "improvement" so a "bugfix" can be backported to release branches. But as LoongArch is still experimental now, I think simply adding "fix the issue ... ..." into the description may be OK.

gonglingqin added inline comments.Nov 22 2022, 2:44 AM
llvm/test/CodeGen/LoongArch/ir-instruction/atomicrmw.ll
685–687

Thanks for your suggestion, I will update the summary.

gonglingqin edited the summary of this revision. (Show Details)Nov 22 2022, 2:45 AM
SixWeining edited the summary of this revision. (Show Details)Nov 22 2022, 3:59 AM
This revision is now accepted and ready to land.Nov 23 2022, 2:11 AM
This revision was landed with ongoing or failed builds.Nov 28 2022, 4:08 AM
This revision was automatically updated to reflect the committed changes.