Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARC][IAS] Add illtrap alias for unimp #105928

Merged
merged 2 commits into from
Aug 29, 2024
Merged

Conversation

koachan
Copy link
Contributor

@koachan koachan commented Aug 24, 2024

This follows Solaris behavior of allowing both mnemonics all the time.

Fixes #105639.

This follows GNU and Solaris behavior of allowing both mnemonics all the time.
@koachan koachan requested review from brad0 and s-barannikov August 24, 2024 07:25
@llvmbot llvmbot added backend:Sparc mc Machine (object) code labels Aug 24, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 24, 2024

@llvm/pr-subscribers-backend-sparc

Author: Koakuma (koachan)

Changes

This follows GNU and Solaris behavior of allowing both mnemonics all the time.

Fixes #105639.


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

2 Files Affected:

  • (modified) llvm/lib/Target/Sparc/SparcInstrAliases.td (+4)
  • (modified) llvm/test/MC/Sparc/sparc-misc-instructions.s (+6)
diff --git a/llvm/lib/Target/Sparc/SparcInstrAliases.td b/llvm/lib/Target/Sparc/SparcInstrAliases.td
index e77b7f5c376bf4..722e0463b7bcaa 100644
--- a/llvm/lib/Target/Sparc/SparcInstrAliases.td
+++ b/llvm/lib/Target/Sparc/SparcInstrAliases.td
@@ -601,6 +601,10 @@ def : InstAlias<"flush", (FLUSH), 0>;
 // unimp -> unimp 0
 def : InstAlias<"unimp", (UNIMP 0), 0>;
 
+// Not in spec, but we follow GNU & Solaris behavior of having `illtrap`
+// interchangeable with `unimp` all the time.
+def : MnemonicAlias<"illtrap", "unimp">;
+
 def : MnemonicAlias<"iflush", "flush">;
 
 def : MnemonicAlias<"stub", "stb">;
diff --git a/llvm/test/MC/Sparc/sparc-misc-instructions.s b/llvm/test/MC/Sparc/sparc-misc-instructions.s
index 0547575eb3db51..8119088cd0dbb1 100644
--- a/llvm/test/MC/Sparc/sparc-misc-instructions.s
+++ b/llvm/test/MC/Sparc/sparc-misc-instructions.s
@@ -6,3 +6,9 @@
 
         ! CHECK: unimp 0   ! encoding: [0x00,0x00,0x00,0x00]
         unimp 0
+
+        ! CHECK: unimp 0   ! encoding: [0x00,0x00,0x00,0x00]
+        illtrap
+
+        ! CHECK: unimp 0   ! encoding: [0x00,0x00,0x00,0x00]
+        illtrap 0

@llvmbot
Copy link
Member

llvmbot commented Aug 24, 2024

@llvm/pr-subscribers-mc

Author: Koakuma (koachan)

Changes

This follows GNU and Solaris behavior of allowing both mnemonics all the time.

Fixes #105639.


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

2 Files Affected:

  • (modified) llvm/lib/Target/Sparc/SparcInstrAliases.td (+4)
  • (modified) llvm/test/MC/Sparc/sparc-misc-instructions.s (+6)
diff --git a/llvm/lib/Target/Sparc/SparcInstrAliases.td b/llvm/lib/Target/Sparc/SparcInstrAliases.td
index e77b7f5c376bf4..722e0463b7bcaa 100644
--- a/llvm/lib/Target/Sparc/SparcInstrAliases.td
+++ b/llvm/lib/Target/Sparc/SparcInstrAliases.td
@@ -601,6 +601,10 @@ def : InstAlias<"flush", (FLUSH), 0>;
 // unimp -> unimp 0
 def : InstAlias<"unimp", (UNIMP 0), 0>;
 
+// Not in spec, but we follow GNU & Solaris behavior of having `illtrap`
+// interchangeable with `unimp` all the time.
+def : MnemonicAlias<"illtrap", "unimp">;
+
 def : MnemonicAlias<"iflush", "flush">;
 
 def : MnemonicAlias<"stub", "stb">;
diff --git a/llvm/test/MC/Sparc/sparc-misc-instructions.s b/llvm/test/MC/Sparc/sparc-misc-instructions.s
index 0547575eb3db51..8119088cd0dbb1 100644
--- a/llvm/test/MC/Sparc/sparc-misc-instructions.s
+++ b/llvm/test/MC/Sparc/sparc-misc-instructions.s
@@ -6,3 +6,9 @@
 
         ! CHECK: unimp 0   ! encoding: [0x00,0x00,0x00,0x00]
         unimp 0
+
+        ! CHECK: unimp 0   ! encoding: [0x00,0x00,0x00,0x00]
+        illtrap
+
+        ! CHECK: unimp 0   ! encoding: [0x00,0x00,0x00,0x00]
+        illtrap 0

@koachan
Copy link
Contributor Author

koachan commented Aug 24, 2024

CC-ing @alexrp too.

@s-barannikov
Copy link
Contributor

It looks like GNU as doesn't allow it when targeting V8:

test.s: Assembler messages:
test.s:2: Error: Architecture mismatch on "illtrap ".
test.s:2: (Requires v9|v9a|v9b|v9c|v9d|v9e|v9v|v9m|m8; requested architecture is v8.)

@s-barannikov
Copy link
Contributor

@koachan
Copy link
Contributor Author

koachan commented Aug 24, 2024

It looks like GNU as doesn't allow it when targeting V8:

test.s: Assembler messages:
test.s:2: Error: Architecture mismatch on "illtrap ".
test.s:2: (Requires v9|v9a|v9b|v9c|v9d|v9e|v9v|v9m|m8; requested architecture is v8.)

Oh, interesting. Seems like using unimp in V9 is fine but using illtrap in V8 is not.

On the other hand Solaris does allow both mnemonics even when explicitly targeting V8:

$ cat unimp.s
u:
 unimp
 unimp 0
 illtrap
 illtrap 0
 retl
  nop
$ as -m32 -xarch=v8 unimp.s && dis unimp.o
disassembly for unimp.o


section .text
    0x0:                    00 00 00 00  unimp     0x0
    0x4:                    00 00 00 00  unimp     0x0
    0x8:                    00 00 00 00  unimp     0x0
    0xc:                    00 00 00 00  unimp     0x0
    0x10:                   81 c3 e0 08  retl
    0x14:                   01 00 00 00  nop

I'm slightly leaning towards using Solaris behavior in this particular case but in any case either choice is okay by me.

@s-barannikov
Copy link
Contributor

I'm slightly leaning towards using Solaris behavior in this particular case but in any case either choice is okay by me.

I'm not sure we should be looking at the Solaris toolchain at all. llvm-based toolchain isn't a drop-in replacement for it.

@koachan koachan merged commit 7955760 into llvm:main Aug 29, 2024
6 of 8 checks passed
@koachan koachan deleted the sparc64-ias branch August 29, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:Sparc mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SPARC] Assembler does not recognize illtrap
3 participants