[llvm] [X86] Don't assume the FPCLASS category mask is a literal (PR #213171)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-x86

Author: Zane Hambly (Zaneham)

<details>
<summary>Changes</summary>

`llvm-mc` asserts on a FPCLASS category mask given as a symbol:

```asm
vfpclassps $f0, %zmm1, %k1
```
```
Assertion failed: isImm() && "This is not an immediate", MCInst.h:85
```

`printFPCLASSComments` reads the last operand with `getImm()` without checking it is one. The value is only known at link time, so there is no category to describe; return early and print no comment.

Note this is a comment printer, so encoding is unaffected. `--filetype=obj` already succeeds today and emits a placeholder immediate plus an `R_X86_64_8` relocation, which is correct. The issues describe this as producing a wrong encoding in release builds, which I don't think is right for these two; the assert only fires when printing assembly.

A literal category still prints its comment as before, covered in the test.

Fixes #<!-- -->185364
Fixes #<!-- -->185365

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


2 Files Affected:

- (modified) llvm/lib/Target/X86/MCTargetDesc/X86InstComments.cpp (+5) 
- (added) llvm/test/MC/X86/symbolic-fpclass-imm.s (+21) 


``````````diff
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86InstComments.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86InstComments.cpp
index d7f21fe17bfe1..0ee814e786b44 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86InstComments.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86InstComments.cpp
@@ -1025,6 +1025,11 @@ static bool printFPCLASSComments(const MCInst *MI, raw_ostream &OS,
   default:
     return false;
   }
+  // The category mask can be a symbol rather than a literal, in which case its
+  // value is only known at link time and there is no category to describe.
+  if (!MI->getOperand(NumOperands - 1).isImm())
+    return false;
+
   StringRef DestName = getRegName(MI->getOperand(0).getReg());
   StringRef SrcName =
       SrcIdx != -1 ? getRegName(MI->getOperand(SrcIdx).getReg()) : "mem";
diff --git a/llvm/test/MC/X86/symbolic-fpclass-imm.s b/llvm/test/MC/X86/symbolic-fpclass-imm.s
new file mode 100644
index 0000000000000..8f4b70dbbdded
--- /dev/null
+++ b/llvm/test/MC/X86/symbolic-fpclass-imm.s
@@ -0,0 +1,21 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -mattr=+avx512dq,+avx512vl %s | FileCheck %s
+// RUN: llvm-mc -triple x86_64-unknown-unknown -mattr=+avx512dq,+avx512vl --show-encoding %s | FileCheck %s --check-prefix=ENC
+
+// The FPCLASS category mask may be a symbol, whose value is only known at link
+// time. The instruction comment printer used to assume it was always a literal
+// and assert.
+
+// CHECK: vfpclassps $f0, %zmm1, %k1{{$}}
+// ENC: vfpclassps $f0, %zmm1, %k1
+// ENC-SAME: encoding: [0x62,0xf3,0x7d,0x48,0x66,0xc9,A]
+vfpclassps $f0, %zmm1, %k1
+
+// A literal category is still described as before.
+
+// CHECK: vfpclassps $3, %zmm1, %k1
+// CHECK-SAME: k1 = isQuietNaN(zmm1) | isPositiveZero(zmm1)
+vfpclassps $3, %zmm1, %k1
+
+// CHECK: vfpclassps $0, %zmm1, %k1
+// CHECK-SAME: k1 = false
+vfpclassps $0, %zmm1, %k1

``````````

</details>


https://github.com/llvm/llvm-project/pull/213171