Skip to content

Conversation

@jthackray
Copy link
Contributor

@jthackray jthackray commented May 30, 2025

The Arm Architecture Reference Manual (DDI 0487K.a) says that:

  • GCSPUSHX
  • GCSPOPCX
  • GCSPOPX

instructions, which are SYS aliases, can take an optional register, e.g. GCSPUSHX {<Xt>}

Fix the code and tests to allow the optional register to be encoded.

…ster

The Arm Archicture Reference Manual (DDI 0487K.a) says that:
  * `GCSPUSHX`
  * `GCSPOPCX`
  * `GCSPOPX`
instructions, which are `SYS` aliases, can take an optional register,
e.g. `GCSPUSHX {<Xt>}`

Fix the code and tests to allow this to be encoded.
@llvmbot llvmbot added backend:AArch64 llvm:mc Machine (object) code labels May 30, 2025
@llvmbot
Copy link
Member

llvmbot commented May 30, 2025

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-aarch64

Author: Jonathan Thackray (jthackray)

Changes

The Arm Archicture Reference Manual (DDI 0487K.a) says that:

  • GCSPUSHX
  • GCSPOPCX
  • GCSPOPX instructions, which are SYS aliases, can take an optional register, e.g. GCSPUSHX {&lt;Xt&gt;}

Fix the code and tests to allow this to be encoded.


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

3 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.td (+10-11)
  • (modified) llvm/test/MC/AArch64/armv9.4a-gcs.s (+12)
  • (modified) llvm/test/MC/Disassembler/AArch64/armv9.4a-gcs.txt (+9)
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.td b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
index 72445172059bf..b1ab0175b531f 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -1537,16 +1537,6 @@ def : TokenAlias<"IALL", "iall">;
 
 
 // ARMv9.4-A Guarded Control Stack
-class GCSNoOp<bits<3> op2, string mnemonic>
-    : SimpleSystemI<0, (ins), mnemonic, "">, Sched<[]> {
-  let Inst{20-8} = 0b0100001110111;
-  let Inst{7-5} = op2;
-  let Predicates = [HasGCS];
-}
-def GCSPUSHX : GCSNoOp<0b100, "gcspushx">;
-def GCSPOPCX : GCSNoOp<0b101, "gcspopcx">;
-def GCSPOPX  : GCSNoOp<0b110, "gcspopx">;
-
 class GCSRtIn<bits<3> op1, bits<3> op2, string mnemonic,
             list<dag> pattern = []>
     : RtSystemI<0, (outs), (ins GPR64:$Rt), mnemonic, "\t$Rt", pattern> {
@@ -1560,8 +1550,17 @@ class GCSRtIn<bits<3> op1, bits<3> op2, string mnemonic,
 
 let mayStore = 1, mayLoad = 1 in
 def GCSSS1   : GCSRtIn<0b011, 0b010, "gcsss1">;
-let mayStore = 1 in
+
+let mayStore = 1 in {
 def GCSPUSHM : GCSRtIn<0b011, 0b000, "gcspushm">;
+def GCSPUSHX : GCSRtIn<0b000, 0b100, "gcspushx">;
+def GCSPOPCX : GCSRtIn<0b000, 0b101, "gcspopcx">;
+def GCSPOPX  : GCSRtIn<0b000, 0b110, "gcspopx">;
+}
+
+def GCSPUSHX_NoOp : InstAlias<"gcspushx", (GCSPUSHX XZR)>, Requires<[HasGCS]>; // Rt defaults to XZR if absent
+def GCSPOPCX_NoOp : InstAlias<"gcspopcx", (GCSPOPCX XZR)>, Requires<[HasGCS]>; // Rt defaults to XZR if absent
+def GCSPOPX_NoOp  : InstAlias<"gcspopx",  (GCSPOPX  XZR)>, Requires<[HasGCS]>; // Rt defaults to XZR if absent
 
 class GCSRtOut<bits<3> op1, bits<3> op2, string mnemonic,
             list<dag> pattern = []>
diff --git a/llvm/test/MC/AArch64/armv9.4a-gcs.s b/llvm/test/MC/AArch64/armv9.4a-gcs.s
index b4af9b5dcb10c..b4b1baba2b788 100644
--- a/llvm/test/MC/AArch64/armv9.4a-gcs.s
+++ b/llvm/test/MC/AArch64/armv9.4a-gcs.s
@@ -113,3 +113,15 @@ gcspopcx
 gcspopx
 // CHECK: gcspopx                           // encoding: [0xdf,0x77,0x08,0xd5]
 // ERROR-NO-GCS: [[@LINE-2]]:1: error: instruction requires: gcs
+
+gcspushx x3
+// CHECK: gcspushx x3                       // encoding: [0x83,0x77,0x08,0xd5]
+// ERROR-NO-GCS: [[@LINE-2]]:1: error: instruction requires: gcs
+
+gcspopcx x3
+// CHECK: gcspopcx x3                       // encoding: [0xa3,0x77,0x08,0xd5]
+// ERROR-NO-GCS: [[@LINE-2]]:1: error: instruction requires: gcs
+
+gcspopx x3
+// CHECK: gcspopx x3                        // encoding: [0xc3,0x77,0x08,0xd5]
+// ERROR-NO-GCS: [[@LINE-2]]:1: error: instruction requires: gcs
diff --git a/llvm/test/MC/Disassembler/AArch64/armv9.4a-gcs.txt b/llvm/test/MC/Disassembler/AArch64/armv9.4a-gcs.txt
index 512f4027d9761..7ad0931d505f5 100644
--- a/llvm/test/MC/Disassembler/AArch64/armv9.4a-gcs.txt
+++ b/llvm/test/MC/Disassembler/AArch64/armv9.4a-gcs.txt
@@ -88,3 +88,12 @@
 
 [0xdf,0x77,0x08,0xd5]
 // CHECK: gcspopx
+
+[0x83,0x77,0x08,0xd5]
+// CHECK: gcspushx x3
+
+[0xa3,0x77,0x08,0xd5]
+// CHECK: gcspopcx x3
+
+[0xc3,0x77,0x08,0xd5]
+// CHECK: gcspopx x3

@jthackray
Copy link
Contributor Author

It appears that the published Arm ARM that I have, DDI 0487K.a is actually wrong, and those instructions don't have an optional register operand. This has been fixed in Arm ARM DDI 0487L.

Therefore rejecting this PR.

@jthackray
Copy link
Contributor Author

Rejecting this PR.

@jthackray jthackray closed this May 30, 2025
@jthackray jthackray deleted the users/jthackray/fix-gcs-instructions branch August 20, 2025 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:AArch64 llvm:mc Machine (object) code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants