[PATCH] D19651: [mips][atomics] Fix partword atomic binary operation implementation

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 28 08:25:38 PDT 2016


dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

LGTM with a couple nits.

Also, does this fix any of the machine verifier failures that Quentin reported in https://llvm.org/bugs/show_bug.cgi?id=27458? I think it might fix atomic.ll or atomicops.ll.

> From the commit message:

>  This can be fixed by applying similar changes as http://reviews.llvm.org/D18995, so that a full 64bit pointer is loaded.


Could you refer to the svn revision rather than the phabricator review?


================
Comment at: test/CodeGen/Mips/atomicrmw.ll:8-19
@@ +7,13 @@
+
+define i16 @f(i16* %t, i16 zeroext %v) {
+; ALL-LABEL: f
+entry:
+; PTR32: lw $[[R0:[0-9]+]]
+; PTR64: ld $[[R0:[0-9]+]]
+
+; ALL: ll ${{[0-9]+}}, 0($[[R0]])
+
+   %t.addr = alloca i16*, align 8
+   %ret = atomicrmw add i16* %t, i16 %v monotonic
+  ret i16 %ret
+}
----------------
This test looks very similar to AtomicLoadAdd16 in atomic.ll can we add checks to that instead? The new 'daddiu ..., $zero, -4' will pass the check in atomic.ll by accident at the moment since `CHECK: addiu` will match inside `daddiu`

================
Comment at: test/CodeGen/Mips/atomicrmw.ll:16
@@ +15,3 @@
+
+   %t.addr = alloca i16*, align 8
+   %ret = atomicrmw add i16* %t, i16 %v monotonic
----------------
This statement is dead


http://reviews.llvm.org/D19651





More information about the llvm-commits mailing list