[PATCH] D138481: [LoongArch] Add atomic ordering information for binary atomic operations

Tue Nov 22 02:44:09 PST 2022

gonglingqin added inline comments.

================
Comment at: llvm/test/CodeGen/LoongArch/ir-instruction/atomicrmw.ll:685
 ; LA32-NEXT:    dbar 0
-; LA32-NEXT:    ll.w $a2, $a1, 0
-; LA32-NEXT:    and $a3, $a2, $a0
-; LA32-NEXT:    sc.w $a3, $a1, 0
+; LA32-NEXT:    ll.w $a2, $a0, 0
+; LA32-NEXT:    and $a3, $a2, $a1
----------------
xry111 wrote:
> gonglingqin wrote:
> > xry111 wrote:
> > > I mean, why this is changed?
> > 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?
> 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.
Thanks for your suggestion, I will update the summary.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138481/new/

https://reviews.llvm.org/D138481