[AArch64][CodeGen] Fix illegal register aliasing bug for mops instrs

[nasherm]

A bug was found where mops instructions were being generated that aliased the source and size registers. This is unpredictable behaviour. This patch usess the earlyclobber constraint on the input source register so that it doesn't alias with the size register. Also a test is introduced which tests affected instructions can't violate this constraint.

[llvmbot]

@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-mc

Author: Nashe Mncube (nasherm)

Changes

A bug was found where mops instructions were being generated that aliased the source and size registers. This is unpredictable behaviour. This patch usess the earlyclobber constraint on the input source register so that it doesn't alias with the size register. Also a test is introduced which tests affected instructions can't violate this constraint.

---

Full diff: https://github.com/llvm/llvm-project/pull/88869.diff

2 Files Affected:

• (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.td (+1-1)
• (added) llvm/test/MC/AArch64/armv9.3a-mops-register-aliasing.s (+15)

diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
index e1624f70185e1e..3bf90778363c6c 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -9486,7 +9486,7 @@ let Predicates = [HasMOPS], Defs = [NZCV], Size = 12, mayStore = 1 in {
   let mayLoad = 0 in {
     def MOPSMemorySetPseudo  : Pseudo<(outs GPR64common:$Rd_wb, GPR64:$Rn_wb),
                                       (ins GPR64common:$Rd, GPR64:$Rn, GPR64:$Rm),
-                                      [], "$Rd = $Rd_wb,$Rn = $Rn_wb">, Sched<[]>;
+                                      [], "$Rd = $Rd_wb,$Rn = $Rn_wb,@earlyclobber $Rn_wb">, Sched<[]>;
   }
 }
 let Predicates = [HasMOPS, HasMTE], Defs = [NZCV], Size = 12, mayLoad = 0, mayStore = 1 in {
diff --git a/llvm/test/MC/AArch64/armv9.3a-mops-register-aliasing.s b/llvm/test/MC/AArch64/armv9.3a-mops-register-aliasing.s
new file mode 100644
index 00000000000000..109b33857a9d2d
--- /dev/null
+++ b/llvm/test/MC/AArch64/armv9.3a-mops-register-aliasing.s
@@ -0,0 +1,15 @@
+// RUN: not llvm-mc -triple aarch64 -mattr=+mops < %s 2>&1 | FileCheck %s
+
+  setp  [x0]!, x1!, x1
+  setm  [x0]!, x1!, x1
+  sete  [x0]!, x1!, x1
+
+// CHECK:      error: invalid SET instruction, source and size registers are the same
+// CHECK-NEXT:   setp  [x0]!, x1!, x1
+// CHECK-NEXT:         ^
+// CHECK-NEXT: error: invalid SET instruction, source and size registers are the same
+// CHECK-NEXT:   setm  [x0]!, x1!, x1
+// CHECK-NEXT:         ^
+// CHECK-NEXT: error: invalid SET instruction, source and size registers are the same
+// CHECK-NEXT:   sete  [x0]!, x1!, x1
+// CHECK-NEXT:         ^

[Stylie777]

Are there any LLVM IR tests that can be added for this? As this is a CodeGen issue we should add these.

[nasherm]

Are there any LLVM IR tests that can be added for this? As this is a CodeGen issue we should add these.

Is the llvm-mc test not enough? My thinking is that an IR test obfuscates what the actual issue is which shows up much later in the compiler backend

[Stylie777]

I think there should also be some LLVM-Lit tests that check the IR CodeGen. The llvm-mc test checks any assembly which the user inputs into the compiler, but if an ACLE Intrinsic is used this can be checked differently within the tests.

The tests can be written in a way that the output is check not to be a certain value/register. See #77770 for an example of how a test can ensure early-clobber was being respected within IR and CodeGen.

[nasherm]

I think there should also be some LLVM-Lit tests that check the IR CodeGen. The llvm-mc test checks any assembly which the user inputs into the compiler, but if an ACLE Intrinsic is used this can be checked differently within the tests.

The tests can be written in a way that the output is check not to be a certain value/register. See #77770 for an example of how a test can ensure early-clobber was being respected within IR and CodeGen.

I've written an IR test and updated the PR

[Stylie777]

One more comment from me.

[Stylie777]

Approved - But worth getting a 2nd set of eyes over it.

[ostannard]

LGTM