[PATCH] D32763: [PPC] Lower load acquire/seq_cst trailing fence to cmp + bne + isync.

[Kit Barton via Phabricator via llvm-commits]

kbarton added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstr64Bit.td:967
+
+let isBarrier = 1, isPseudo = 1, Defs = [CR7], Itinerary = IIC_LdStSync in
+def CFENCE8 : Pseudo<(outs), (ins g8rc:$cr), "#CFENCE8", []>;
----------------
Do we also need to mark Uses = [CR7]?


================
Comment at: llvm/test/CodeGen/PowerPC/atomic-2.ll:112
 ; CHECK: ld
-; CHECK: lwsync
+; CHECK: cmpw
+; CHECK: bne-
----------------
Please add a reg expression here to ensure the registers used by the compare are the same as the ones in the ld above (this is necessary for correctness of the isync). 


================
Comment at: llvm/test/CodeGen/PowerPC/atomic-2.ll:114
+; CHECK: bne-
+; CHECK: isync
   ret i64 %tmp
----------------
Please add CHECK-NEXT for the bne- and isync instructions. We want to ensure the cmp, bc, isync instructions stay together. 


================
Comment at: llvm/test/CodeGen/PowerPC/atomics-indexed.ll:15
+; CHECK-PPC32: lwsync
+; CHECK-PPC64: cmpw
+; CHECK-PPC64: bne-
----------------
Same comments as above for reg expression and CHECK-NEXT


================
Comment at: llvm/test/CodeGen/PowerPC/atomics-indexed.ll:26
+; CHECK-PPC32: lwsync
+; CHECK-PPC64: cmpw
+; CHECK-PPC64: bne-
----------------
And here as well


================
Comment at: llvm/test/CodeGen/PowerPC/atomics.ll:31
+; CHECK-PPC32: lwsync
+; CHECK-PPC64: cmpw
+; CHECK-PPC64: bne-
----------------
And here as well.


================
Comment at: llvm/test/CodeGen/PowerPC/atomics.ll:44
+; CHECK-PPC32: lwsync
+; CHECK-PPC64: cmpw
+; CHECK-PPC64: bne-
----------------
And last but not least... :)


https://reviews.llvm.org/D32763