Skip to content

[AMDGPU] Change getLdStRegisterOperand to !cond for better diagnostic #95475

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

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

Sisyph
Copy link
Contributor

@Sisyph Sisyph commented Jun 13, 2024

If you would hit the unexpected case in these !if trees, you'd get an error message like "error: Not a known RegisterClass! def VReg_1..." This can happen when changing code quite indirectly related to these class definitions. We can use !cond here, which has a builtin facility to throw an error if no case in the !cond statement is hit.

NFC.

If you would hit the unexpected case in these !if trees, you'd get an
error message like "error: Not a known RegisterClass! def VReg_1..."
This can happen when changing code quite indirectly related to these class
definitions. We can use !cond here, which has a builtin facility to
throw an error if no case in the !cond statement is hit.

NFC.
@llvmbot
Copy link
Member

llvmbot commented Jun 13, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Joe Nash (Sisyph)

Changes

If you would hit the unexpected case in these !if trees, you'd get an error message like "error: Not a known RegisterClass! def VReg_1..." This can happen when changing code quite indirectly related to these class definitions. We can use !cond here, which has a builtin facility to throw an error if no case in the !cond statement is hit.

NFC.


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

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/BUFInstructions.td (+4-6)
  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.td (+5-7)
diff --git a/llvm/lib/Target/AMDGPU/BUFInstructions.td b/llvm/lib/Target/AMDGPU/BUFInstructions.td
index 50e62788c5eac..43e5434ea2700 100644
--- a/llvm/lib/Target/AMDGPU/BUFInstructions.td
+++ b/llvm/lib/Target/AMDGPU/BUFInstructions.td
@@ -399,12 +399,10 @@ class MUBUF_Invalidate <string opName, SDPatternOperator node = null_frag> :
 
 class getLdStVDataRegisterOperand<RegisterClass RC, bit isTFE> {
   RegisterOperand tfeVDataOp =
-    !if(!eq(RC.Size, 32), AVLdSt_64,
-    !if(!eq(RC.Size, 64), AVLdSt_96,
-    !if(!eq(RC.Size, 96), AVLdSt_128,
-    !if(!eq(RC.Size, 128), AVLdSt_160,
-    RegisterOperand<VReg_1>  // Invalid register.
-    ))));
+    !cond(!eq(RC.Size, 32)  : AVLdSt_64,
+          !eq(RC.Size, 64)  : AVLdSt_96,
+          !eq(RC.Size, 96)  : AVLdSt_128,
+          !eq(RC.Size, 128) : AVLdSt_160);
 
   RegisterOperand ret = !if(isTFE, tfeVDataOp, getLdStRegisterOperand<RC>.ret);
 }
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.td b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
index 3921b1469e15e..6682763210411 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.td
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
@@ -2242,13 +2242,11 @@ class getHasExt <int NumSrcArgs, ValueType DstVT = i32, ValueType Src0VT = i32,
 // Return an AGPR+VGPR operand class for the given VGPR register class.
 class getLdStRegisterOperand<RegisterClass RC> {
   RegisterOperand ret =
-    !if(!eq(RC.Size, 32), AVLdSt_32,
-      !if(!eq(RC.Size, 64), AVLdSt_64,
-        !if(!eq(RC.Size, 96), AVLdSt_96,
-          !if(!eq(RC.Size, 128), AVLdSt_128,
-            !if(!eq(RC.Size, 160), AVLdSt_160,
-              RegisterOperand<VReg_1> // invalid register
-    )))));
+    !cond(!eq(RC.Size, 32)   : AVLdSt_32,
+          !eq(RC.Size, 64)   : AVLdSt_64,
+          !eq(RC.Size, 96)   : AVLdSt_96,
+          !eq(RC.Size, 128)  : AVLdSt_128,
+          !eq(RC.Size, 160)  : AVLdSt_160);
 }
 
 class getHasVOP3DPP <ValueType DstVT = i32, ValueType Src0VT = i32,

@Sisyph Sisyph merged commit 7e3e9d4 into llvm:main Jun 14, 2024
9 checks passed
@Sisyph Sisyph deleted the main_vreg1_error branch June 14, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants