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

AMDGPU: Fix assembler asserting on expressions in vop3 instructions #100103

Merged
merged 2 commits into from
Jul 23, 2024

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Jul 23, 2024

Fixes #100075

Copy link
Contributor Author

arsenm commented Jul 23, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @arsenm and the rest of your teammates on Graphite Graphite

@arsenm arsenm force-pushed the users/arsenm/amdgpu-fix-issue100075-asm-expr-operand branch from f9e02fb to 0510315 Compare July 23, 2024 11:03
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 23, 2024

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

Fixes #100075


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

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp (+1-1)
  • (added) llvm/test/MC/AMDGPU/reloc-operands-gfx10.s (+31)
diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index 92c3b26ca4d6f..9616a0ff57134 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -8649,7 +8649,7 @@ void AMDGPUAsmParser::cvtVOP3(MCInst &Inst, const OperandVector &Operands,
       Op.addRegOrImmWithFPInputModsOperands(Inst, 2);
     } else if (Op.isImmModifier()) {
       OptionalIdx[Op.getImmTy()] = I;
-    } else if (Op.isRegOrImm()) {
+    } else if (Op.isRegOrImm() || Op.isExpr()) {
       Op.addRegOrImmOperands(Inst, 1);
     } else {
       llvm_unreachable("unhandled operand type");
diff --git a/llvm/test/MC/AMDGPU/reloc-operands-gfx10.s b/llvm/test/MC/AMDGPU/reloc-operands-gfx10.s
new file mode 100644
index 0000000000000..b0fe71ddadb3b
--- /dev/null
+++ b/llvm/test/MC/AMDGPU/reloc-operands-gfx10.s
@@ -0,0 +1,31 @@
+// RUN: llvm-mc -triple=amdgcn -mcpu=gfx1010 -show-encoding %s | FileCheck -check-prefix=GFX10 %s
+
+; test vop3 operands
+
+// GFX10: v_mad_u32_u24 v0, g0@abs32@lo, v0, 12   ; encoding: [0x00,0x00,0x43,0xd5,0xff,0x00,0x32,0x02,A,A,A,A]
+// GFX10-NEXT:                                    ;   fixup A - offset: 8, value: g0@abs32@lo, kind: FK_Data_4
+v_mad_u32_u24 v0, g0@abs32@lo, v0, 12
+
+// GFX10: v_mad_u32_u24 v0, v0, g0@abs32@lo, 12   ; encoding: [0x00,0x00,0x43,0xd5,0x00,0xff,0x31,0x02,A,A,A,A]
+// GFX10-NEXT:                                    ;   fixup A - offset: 8, value: g0@abs32@lo, kind: FK_Data_4
+v_mad_u32_u24 v0, v0, g0@abs32@lo, 12
+
+// GFX10: v_mad_u32_u24 v0, v0, 12, g0@abs32@lo   ; encoding: [0x00,0x00,0x43,0xd5,0x00,0x19,0xfd,0x03,A,A,A,A]
+// GFX10-NEXT:                                    ;   fixup A - offset: 8, value: g0@abs32@lo, kind: FK_Data_4
+v_mad_u32_u24 v0, v0, 12, g0@abs32@lo
+
+; test vop2 operands
+
+// GFX10: v_add_nc_u32_e32 v0, g0@abs32@lo, v1    ; encoding: [0xff,0x02,0x00,0x4a,A,A,A,A]
+// GFX10-NEXT:                                    ;   fixup A - offset: 4, value: g0@abs32@lo, kind: FK_Data_4
+v_add_nc_u32 v0, g0@abs32@lo, v1
+
+// GFX10: v_add_nc_u32_e64 v0, v1, g0@abs32@lo    ; encoding: [0x00,0x00,0x25,0xd5,0x01,0xff,0x01,0x00,A,A,A,A]
+// GFX10-NEXT:                                    ;   fixup A - offset: 8, value: g0@abs32@lo, kind: FK_Data_4
+v_add_nc_u32 v0, v1, g0@abs32@lo
+
+// test vop1 operands
+// GFX10: v_not_b32_e32 v0, g0@abs32@lo           ; encoding: [0xff,0x6e,0x00,0x7e,A,A,A,A]
+// GFX10-NEXT:                                    ;   fixup A - offset: 4, value: g0@abs32@lo, kind: FK_Data_4
+v_not_b32 v0, g0@abs32@lo
+

@arsenm arsenm marked this pull request as ready for review July 23, 2024 11:05
@llvmbot llvmbot added the mc Machine (object) code label Jul 23, 2024
Copy link
Contributor

@jayfoad jayfoad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with or without the suggestion - thanks!

@arsenm arsenm merged commit 81e2a57 into main Jul 23, 2024
7 checks passed
@arsenm arsenm deleted the users/arsenm/amdgpu-fix-issue100075-asm-expr-operand branch July 23, 2024 14:58
sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…100103)

Summary:
Fixes #100075

---------

Co-authored-by: Jay Foad <[email protected]>

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251321
@jayfoad
Copy link
Contributor

jayfoad commented Aug 8, 2024

Is this worth backporting to 19.x?

@arsenm
Copy link
Contributor Author

arsenm commented Aug 8, 2024

Is this worth backporting to 19.x?

Sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[AMDGPU] Assembler cannot handle v_mad_u32_u24 with relocated operand
4 participants