[PATCH] D14397: [mips] Define patterns for the atomic_{load, store}_{8, 16, 32, 64} nodes.

[Daniel Sanders via llvm-commits]

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

LGTM with some minor changes


================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:394
@@ -393,5 +393,3 @@
 
-  setOperationAction(ISD::ATOMIC_LOAD,       MVT::i32,    Expand);
-  setOperationAction(ISD::ATOMIC_LOAD,       MVT::i64,    Expand);
-  setOperationAction(ISD::ATOMIC_STORE,      MVT::i32,    Expand);
-  setOperationAction(ISD::ATOMIC_STORE,      MVT::i64,    Expand);
+  if (!Subtarget.isGP64bit()) {
+    setOperationAction(ISD::ATOMIC_LOAD,     MVT::i64,   Expand);
----------------
This ought to be a check for N32/N64 but I'm planning to change the predicate itself later so no need to change this now.

================
Comment at: lib/Target/Mips/MipsInstrInfo.td:2087-2092
@@ +2086,8 @@
+// Atomic load patterns.
+multiclass AtomicLoadStorePats<PatFrag LdNode, Instruction LdInst,
+                               PatFrag StNode, Instruction StInst,
+                               RegisterClass RC = GPR32> {
+  def : MipsPat<(LdNode addr:$a), (LdInst addr:$a)>;
+  def : MipsPat<(StNode addr:$a, RC:$v), (StInst RC:$v, addr:$a)>;
+}
+
----------------
The multiclass doesn't provide a noticeable benefit here. It's just distributing the RC argument. I'd prefer loads/stores to be defined separately.

================
Comment at: test/CodeGen/Mips/atomic-load-store.ll:1
@@ +1,2 @@
+; RUN: llc -march=mips -mcpu=mips32r2 < %s | FileCheck %s -check-prefix=ALL
+; RUN: llc -march=mips -mcpu=mips32r6 < %s | FileCheck %s -check-prefix=ALL
----------------
(filename): This kind of minimal single-statement test is what I had in mind when creating the test/CodeGen/Mips/llvm-ir directory. Could you split the load/stores into test/CodeGen/Mips/llvm-ir/{store,load}-atomic.ll

================
Comment at: test/CodeGen/Mips/atomicSCr6.ll:1
@@ -1,2 +1,2 @@
-; RUN: llc -asm-show-inst  -march=mips64el -mcpu=mips64r6 < %s -filetype=asm -o - | FileCheck %s -check-prefix=CHK64 
-; RUN: llc -asm-show-inst  -march=mipsel -mcpu=mips32r6 < %s -filetype=asm -o -| FileCheck %s -check-prefix=CHK32
+; RUN: llc -asm-show-inst  -march=mipsel -mcpu=mips32r6 < %s | \
+; RUN:    FileCheck %s -check-prefix=CHK32
----------------
(filename): Similarly here but with atomicrmx.ll

Thanks for tidying this test.


http://reviews.llvm.org/D14397