[PATCH] D128435: [AMDGPU] Fix assertion failure on mad with negative immediate addend

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D128435#3607680 <https://reviews.llvm.org/D128435#3607680>, @foad wrote:

> Alternative quick fix:
>
>   diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
>   index b07c0a67ecf5..bd938d829953 100644
>   --- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
>   +++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
>   @@ -542,7 +542,7 @@ void AMDGPUInstPrinter::printImmediate64(uint64_t Imm,
>               STI.getFeatureBits()[AMDGPU::FeatureInv2PiInlineImm])
>        O << "0.15915494309189532";
>      else {
>   -    assert(isUInt<32>(Imm) || Imm == 0x3fc45f306dc9c882);
>   +    assert(isUInt<32>(Imm) || isInt<32>(Imm));
>    
>        // In rare situations, we will have a 32-bit literal in a 64-bit
>        // operand. This is technically allowed for the encoding of s_mov_b64.
>
> (I don't understand why `Imm == 0x3fc45f306dc9c882` was allowed in the assertion, on targets that don't have FeatureInv2PiInlineImm.)



In D128435#3607680 <https://reviews.llvm.org/D128435#3607680>, @foad wrote:

> Alternative quick fix:
>
>   diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
>   index b07c0a67ecf5..bd938d829953 100644
>   --- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
>   +++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
>   @@ -542,7 +542,7 @@ void AMDGPUInstPrinter::printImmediate64(uint64_t Imm,
>               STI.getFeatureBits()[AMDGPU::FeatureInv2PiInlineImm])
>        O << "0.15915494309189532";
>      else {
>   -    assert(isUInt<32>(Imm) || Imm == 0x3fc45f306dc9c882);
>   +    assert(isUInt<32>(Imm) || isInt<32>(Imm));
>    

This is a better quick fix

> (I don't understand why `Imm == 0x3fc45f306dc9c882` was allowed in the assertion, on targets that don't have FeatureInv2PiInlineImm.)

Probably laziness for querying the feature


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128435/new/

https://reviews.llvm.org/D128435