From e42da35bfc2779a2518818a4735c0f5b460e598f Mon Sep 17 00:00:00 2001
From: Dylan McKay <me@dylanmckay.io>
Date: Thu, 24 Aug 2017 00:14:38 +0000
Subject: [PATCH] [AVR] Use the correct register classes for 16-bit atomic
 operations

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@311620 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Target/AVR/AVRInstrInfo.td                | 64 ++++++++++++-------
 .../load-store-16-unexpected-register-bug.ll  | 23 +++++++
 2 files changed, 65 insertions(+), 22 deletions(-)
 create mode 100644 test/CodeGen/AVR/atomics/load-store-16-unexpected-register-bug.ll

diff --git a/lib/Target/AVR/AVRInstrInfo.td b/lib/Target/AVR/AVRInstrInfo.td
index 184e4d53f7c..61a227b7bda 100644
--- a/lib/Target/AVR/AVRInstrInfo.td
+++ b/lib/Target/AVR/AVRInstrInfo.td
@@ -1238,35 +1238,55 @@ isReMaterializable = 1 in
                  Requires<[HasSRAM]>;
 }
 
-class AtomicLoad<PatFrag Op, RegisterClass DRC> :
-  Pseudo<(outs DRC:$rd), (ins PTRREGS:$rr), "atomic_op",
+class AtomicLoad<PatFrag Op, RegisterClass DRC,
+                 RegisterClass PTRRC> :
+  Pseudo<(outs DRC:$rd), (ins PTRRC:$rr), "atomic_op",
          [(set DRC:$rd, (Op i16:$rr))]>;
 
-class AtomicStore<PatFrag Op, RegisterClass DRC> :
-  Pseudo<(outs), (ins PTRDISPREGS:$rd, DRC:$rr), "atomic_op",
+class AtomicStore<PatFrag Op, RegisterClass DRC,
+                  RegisterClass PTRRC> :
+  Pseudo<(outs), (ins PTRRC:$rd, DRC:$rr), "atomic_op",
          [(Op i16:$rd, DRC:$rr)]>;
 
-class AtomicLoadOp<PatFrag Op, RegisterClass DRC> :
-  Pseudo<(outs DRC:$rd), (ins PTRREGS:$rr, DRC:$operand),
+class AtomicLoadOp<PatFrag Op, RegisterClass DRC,
+                   RegisterClass PTRRC> :
+  Pseudo<(outs DRC:$rd), (ins PTRRC:$rr, DRC:$operand),
          "atomic_op",
          [(set DRC:$rd, (Op i16:$rr, DRC:$operand))]>;
 
-def AtomicLoad8   : AtomicLoad<atomic_load_8, GPR8>;
-def AtomicLoad16  : AtomicLoad<atomic_load_16, DREGS>;
-
-def AtomicStore8  : AtomicStore<atomic_store_8, GPR8>;
-def AtomicStore16 : AtomicStore<atomic_store_16, DREGS>;
-
-def AtomicLoadAdd8  : AtomicLoadOp<atomic_load_add_8, GPR8>;
-def AtomicLoadAdd16 : AtomicLoadOp<atomic_load_add_16, DREGS>;
-def AtomicLoadSub8  : AtomicLoadOp<atomic_load_sub_8, GPR8>;
-def AtomicLoadSub16 : AtomicLoadOp<atomic_load_sub_16, DREGS>;
-def AtomicLoadAnd8  : AtomicLoadOp<atomic_load_and_8, GPR8>;
-def AtomicLoadAnd16 : AtomicLoadOp<atomic_load_and_16, DREGS>;
-def AtomicLoadOr8   : AtomicLoadOp<atomic_load_or_8, GPR8>;
-def AtomicLoadOr16  : AtomicLoadOp<atomic_load_or_16, DREGS>;
-def AtomicLoadXor8  : AtomicLoadOp<atomic_load_xor_8, GPR8>;
-def AtomicLoadXor16 : AtomicLoadOp<atomic_load_xor_16, DREGS>;
+// FIXME: I think 16-bit atomic binary ops need to mark
+// r0 as clobbered.
+
+// Atomic instructions
+// ===================
+//
+// These are all expanded by AVRExpandPseudoInsts
+//
+// 8-bit operations can use any pointer register because
+// they are expanded directly into an LD/ST instruction.
+//
+// 16-bit operations use 16-bit load/store postincrement instructions,
+// which require PTRDISPREGS.
+
+def AtomicLoad8   : AtomicLoad<atomic_load_8, GPR8, PTRREGS>;
+def AtomicLoad16  : AtomicLoad<atomic_load_16, DREGS, PTRDISPREGS>;
+
+def AtomicStore8  : AtomicStore<atomic_store_8, GPR8, PTRREGS>;
+def AtomicStore16 : AtomicStore<atomic_store_16, DREGS, PTRDISPREGS>;
+
+class AtomicLoadOp8<PatFrag Op> : AtomicLoadOp<Op, GPR8, PTRREGS>;
+class AtomicLoadOp16<PatFrag Op> : AtomicLoadOp<Op, DREGS, PTRDISPREGS>;
+
+def AtomicLoadAdd8  : AtomicLoadOp8<atomic_load_add_8>;
+def AtomicLoadAdd16 : AtomicLoadOp16<atomic_load_add_16>;
+def AtomicLoadSub8  : AtomicLoadOp8<atomic_load_sub_8>;
+def AtomicLoadSub16 : AtomicLoadOp16<atomic_load_sub_16>;
+def AtomicLoadAnd8  : AtomicLoadOp8<atomic_load_and_8>;
+def AtomicLoadAnd16 : AtomicLoadOp16<atomic_load_and_16>;
+def AtomicLoadOr8   : AtomicLoadOp8<atomic_load_or_8>;
+def AtomicLoadOr16  : AtomicLoadOp16<atomic_load_or_16>;
+def AtomicLoadXor8  : AtomicLoadOp8<atomic_load_xor_8>;
+def AtomicLoadXor16 : AtomicLoadOp16<atomic_load_xor_16>;
 def AtomicFence     : Pseudo<(outs), (ins), "atomic_fence",
                              [(atomic_fence imm, imm)]>;
 
diff --git a/test/CodeGen/AVR/atomics/load-store-16-unexpected-register-bug.ll b/test/CodeGen/AVR/atomics/load-store-16-unexpected-register-bug.ll
new file mode 100644
index 00000000000..9c72940b2b7
--- /dev/null
+++ b/test/CodeGen/AVR/atomics/load-store-16-unexpected-register-bug.ll
@@ -0,0 +1,23 @@
+; RUN: llc < %s -march=avr | FileCheck %s
+
+; At one point, the 16-vit atomic load/store operations we defined in TableGen
+; to use 'PTRREGS', but the pseudo expander would generate LDDW/STDW instructions.
+;
+; This would sometimes cause codegen to fail because LDDW requires 'PTRDISPREGS', and
+; so if we attempted to generate an atomic operation on the X register, it would hit
+; an assertion;
+
+%AtomicI16 = type { %UnsafeCell, [0 x i8] }
+%UnsafeCell = type { i16, [0 x i8] }
+
+; CHECK-LABEL: foo
+define i8 @foo(%AtomicI16*) {
+start:
+
+; We should not be generating atomics that use the X register, they will fail when emitting MC.
+; CHECK-NOT: X
+  %1 = getelementptr inbounds %AtomicI16, %AtomicI16* %0, i16 0, i32 0, i32 0
+  %2 = load atomic i16, i16* %1 seq_cst, align 2
+  ret i8 0
+}
+
-- 
2.50.1