[Mlir-commits] [mlir] [AMDGPU] Change the representation of double literals in operands (PR #68740)

[Jay Foad]

================
@@ -4309,7 +4312,18 @@ bool AMDGPUAsmParser::validateVOPLiteral(const MCInst &Inst,
       continue;
 
     if (MO.isImm() && !isInlineConstant(Inst, OpIdx)) {
-      uint32_t Value = static_cast<uint32_t>(MO.getImm());
+      uint64_t Value = static_cast<uint64_t>(MO.getImm());
+      bool IsFP = AMDGPU::isSISrcFPOperand(Desc, OpIdx);
+      bool IsValid32Op = AMDGPU::isValid32BitLiteral(Value, IsFP);
+
+      if (!IsValid32Op && !isInt<32>(Value) && !isUInt<32>(Value)) {
+        Error(getLitLoc(Operands), "invalid operand for instruction");
+        return false;
+      }
+
+      if (IsFP && IsValid32Op)
+        Value = Hi_32(Value);
----------------
jayfoad wrote:

I think I have finally understood this code. I would prefer to write it as:
```suggestion
      bool IsFP64 = AMDGPU::isSISrcFPOperand(Desc, OpIdx) && AMDGPU::getOperandSize(Desc.operands()[OpIdx]) == 8;

      if (!AMDGPU::isValid32BitLiteral(Value, IsFP64)) {
        Error(getLitLoc(Operands), "invalid operand for instruction");
        return false;
      }

      if (IsFP64)
        Value = Hi_32(Value);
```
(The call to `getOperandSize` is still pretty ugly. Maybe there is a better way of detecting fp64 operands?)

As a corolloray, maybe change the name of `isValid32BitLiteral`'s second argument from `IsFP` to `IsFP64`? Or maybe even remove that helper function altogether and inline it here? The only other use of it is in an assert where it can just be replaced by `assert(Lo_32(Imm) == 0)`.

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