[llvm] r355836 - [AMDGPU] Mark enum types in SIDefines.h as unsigned

Mikael Holmén via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 13 03:51:31 PDT 2019


And this one also popped up

  Building CXX object 
lib/Target/AMDGPU/Disassembler/CMakeFiles/LLVMAMDGPUDisassembler.dir/AMDGPUDisassembler.cpp.o
../lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp: In member 
function 'llvm::MCOperand 
llvm::AMDGPUDisassembler::decodeSDWASrc(llvm::AMDGPUDisassembler::OpWidthTy, 
unsigned int) const':
../lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:828:38: 
warning: comparison of unsigned expression >= 0 is always true 
[-Wtype-limits]
      if (SDWA9EncValues::SRC_VGPR_MIN <= Val &&
          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~

(Also funny that there is still the XXX comment above the condition 
saying that the static_cast<int> is needed
     // XXX: static_cast<int> is needed to avoid stupid warning:
     // compare with unsigned is always true
     if (SDWA9EncValues::SRC_VGPR_MIN <= Val &&
         Val <= SDWA9EncValues::SRC_VGPR_MAX) {
)

/Mikael

On 3/13/19 11:39 AM, Mikael Holmén via llvm-commits wrote:
> Hi,
> 
> On 3/11/19 5:49 PM, Stanislav Mekhanoshin via llvm-commits wrote:
>> Author: rampitec
>> Date: Mon Mar 11 09:49:32 2019
>> New Revision: 355836
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=355836&view=rev
>> Log:
>> [AMDGPU] Mark enum types in SIDefines.h as unsigned
>>
>> MSVC issues some warnings about signed/unsigned comparison.
> 
> gcc complains too (at least 7.4), e.g.
> 
> ../lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp: In member function
> 'int64_t {anonymous}::AMDGPUOperand::Modifiers::getFPModifiersOperand()
> const':
> ../lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:105:22: warning:
> enumeral and non-enumeral type in conditional expression [-Wextra]
>          Operand |= Abs ? SISrcMods::ABS : 0;
>                     ~~~~^~~~~~~~~~~~~~~~~~~~
> ../lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:106:22: warning:
> enumeral and non-enumeral type in conditional expression [-Wextra]
>          Operand |= Neg ? SISrcMods::NEG : 0;
>                     ~~~~^~~~~~~~~~~~~~~~~~~~
> ../lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp: In member function
> 'int64_t {anonymous}::AMDGPUOperand::Modifiers::getIntModifiersOperand()
> const':
> ../lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:113:23: warning:
> enumeral and non-enumeral type in conditional expression [-Wextra]
>          Operand |= Sext ? SISrcMods::SEXT : 0;
>                     ~~~~~^~~~~~~~~~~~~~~~~~~~~
> 
> ../lib/Target/AMDGPU/SIFoldOperands.cpp: In member function 'const
> llvm::MachineOperand* {anonymous}::SIFoldOperands::isClamp(const
> llvm::MachineInstr&) const':
> ../lib/Target/AMDGPU/SIFoldOperands.cpp:928:55: warning: enumeral and
> non-enumeral type in conditional expression [-Wextra]
>        unsigned UnsetMods = (Op == AMDGPU::V_PK_MAX_F16) ?
> SISrcMods::OP_SEL_1 : 0;
>   
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
> [67/91] Building CXX object
> lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/SIPeepholeSDWA.cpp.o
> ../lib/Target/AMDGPU/SIPeepholeSDWA.cpp: In member function 'uint64_t
> {anonymous}::SDWASrcOperand::getSrcMods(const llvm::SIInstrInfo*, const
> llvm::MachineOperand*) const':
> ../lib/Target/AMDGPU/SIPeepholeSDWA.cpp:350:17: warning: enumeral and
> non-enumeral type in conditional expression [-Wextra]
>        Mods |= Abs ? SISrcMods::ABS : 0;
>                ~~~~^~~~~~~~~~~~~~~~~~~~
> ../lib/Target/AMDGPU/SIPeepholeSDWA.cpp:351:17: warning: enumeral and
> non-enumeral type in conditional expression [-Wextra]
>        Mods ^= Neg ? SISrcMods::NEG : 0;
>                ~~~~^~~~~~~~~~~~~~~~~~~~
> 
> 
> The warnings can be silenced by e.g. doing
> 
>          Operand |= Abs ? SISrcMods::ABS : 0u;
> 
> but I don't know if that the best way to go about it.
> 
> Regards,
> Mikael
> 
>>
>> Differential Revision: https://reviews.llvm.org/D59171
>>
>> Modified:
>>       llvm/trunk/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
>>       llvm/trunk/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
>>       llvm/trunk/lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp
>>       llvm/trunk/lib/Target/AMDGPU/SIDefines.h
>>
>> Modified: llvm/trunk/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp?rev=355836&r1=355835&r2=355836&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp (original)
>> +++ llvm/trunk/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp Mon Mar 11 09:49:32 2019
>> @@ -4490,8 +4490,8 @@ AMDGPUAsmParser::parseSwizzleQuadPerm(in
>>      if (parseSwizzleOperands(LANE_NUM, Lane, 0, LANE_MAX,
>>                               "expected a 2-bit lane id")) {
>>        Imm = QUAD_PERM_ENC;
>> -    for (auto i = 0; i < LANE_NUM; ++i) {
>> -      Imm |= Lane[i] << (LANE_SHIFT * i);
>> +    for (unsigned I = 0; I < LANE_NUM; ++I) {
>> +      Imm |= Lane[I] << (LANE_SHIFT * I);
>>        }
>>        return true;
>>      }
>>
>> Modified: llvm/trunk/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp?rev=355836&r1=355835&r2=355836&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp (original)
>> +++ llvm/trunk/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp Mon Mar 11 09:49:32 2019
>> @@ -825,7 +825,7 @@ MCOperand AMDGPUDisassembler::decodeSDWA
>>      if (STI.getFeatureBits()[AMDGPU::FeatureGFX9]) {
>>        // XXX: static_cast<int> is needed to avoid stupid warning:
>>        // compare with unsigned is always true
>> -    if (SDWA9EncValues::SRC_VGPR_MIN <= static_cast<int>(Val) &&
>> +    if (SDWA9EncValues::SRC_VGPR_MIN <= Val &&
>>            Val <= SDWA9EncValues::SRC_VGPR_MAX) {
>>          return createRegOperand(getVgprClassId(Width),
>>                                  Val - SDWA9EncValues::SRC_VGPR_MIN);
>>
>> Modified: llvm/trunk/lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp?rev=355836&r1=355835&r2=355836&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp (original)
>> +++ llvm/trunk/lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp Mon Mar 11 09:49:32 2019
>> @@ -1097,7 +1097,7 @@ void AMDGPUInstPrinter::printSwizzle(con
>>      if ((Imm & QUAD_PERM_ENC_MASK) == QUAD_PERM_ENC) {
>>    
>>        O << "swizzle(" << IdSymbolic[ID_QUAD_PERM];
>> -    for (auto i = 0; i < LANE_NUM; ++i) {
>> +    for (unsigned I = 0; I < LANE_NUM; ++I) {
>>          O << ",";
>>          O << formatDec(Imm & LANE_MASK);
>>          Imm >>= LANE_SHIFT;
>>
>> Modified: llvm/trunk/lib/Target/AMDGPU/SIDefines.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AMDGPU/SIDefines.h?rev=355836&r1=355835&r2=355836&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/AMDGPU/SIDefines.h (original)
>> +++ llvm/trunk/lib/Target/AMDGPU/SIDefines.h Mon Mar 11 09:49:32 2019
>> @@ -95,7 +95,7 @@ enum : uint64_t {
>>    
>>    // v_cmp_class_* etc. use a 10-bit mask for what operation is checked.
>>    // The result is true if any of these tests are true.
>> -enum ClassFlags {
>> +enum ClassFlags : unsigned {
>>      S_NAN = 1 << 0,        // Signaling NaN
>>      Q_NAN = 1 << 1,        // Quiet NaN
>>      N_INFINITY = 1 << 2,   // Negative infinity
>> @@ -110,7 +110,7 @@ enum ClassFlags {
>>    }
>>    
>>    namespace AMDGPU {
>> -  enum OperandType {
>> +  enum OperandType : unsigned {
>>        /// Operands with register or 32-bit immediate
>>        OPERAND_REG_IMM_INT32 = MCOI::OPERAND_FIRST_TARGET,
>>        OPERAND_REG_IMM_INT64,
>> @@ -160,7 +160,7 @@ enum StackTypes : uint8_t {
>>    // Input operand modifiers bit-masks
>>    // NEG and SEXT share same bit-mask because they can't be set simultaneously.
>>    namespace SISrcMods {
>> -  enum {
>> +  enum : unsigned {
>>       NEG = 1 << 0,   // Floating-point negate modifier
>>       ABS = 1 << 1,   // Floating-point absolute modifier
>>       SEXT = 1 << 0,  // Integer sign-extend modifier
>> @@ -172,7 +172,7 @@ namespace SISrcMods {
>>    }
>>    
>>    namespace SIOutMods {
>> -  enum {
>> +  enum : unsigned {
>>        NONE = 0,
>>        MUL2 = 1,
>>        MUL4 = 2,
>> @@ -183,7 +183,7 @@ namespace SIOutMods {
>>    namespace AMDGPU {
>>    namespace VGPRIndexMode {
>>    
>> -enum Id { // id of symbolic names
>> +enum Id : unsigned { // id of symbolic names
>>      ID_SRC0 = 0,
>>      ID_SRC1,
>>      ID_SRC2,
>> @@ -193,7 +193,7 @@ enum Id { // id of symbolic names
>>      ID_MAX = ID_DST
>>    };
>>    
>> -enum EncBits {
>> +enum EncBits : unsigned {
>>      OFF = 0,
>>      SRC0_ENABLE = 1 << ID_SRC0,
>>      SRC1_ENABLE = 1 << ID_SRC1,
>> @@ -206,7 +206,7 @@ enum EncBits {
>>    } // namespace AMDGPU
>>    
>>    namespace AMDGPUAsmVariants {
>> -  enum {
>> +  enum : unsigned {
>>        DEFAULT = 0,
>>        VOP3 = 1,
>>        SDWA = 2,
>> @@ -218,7 +218,7 @@ namespace AMDGPUAsmVariants {
>>    namespace AMDGPU {
>>    namespace EncValues { // Encoding values of enum9/8/7 operands
>>    
>> -enum {
>> +enum : unsigned {
>>      SGPR_MIN = 0,
>>      SGPR_MAX = 101,
>>      TTMP_VI_MIN = 112,
>> @@ -277,7 +277,7 @@ enum Op { // Both GS and SYS operation I
>>      OP_SYS_MASK_ = (((1 << OP_SYS_WIDTH_) - 1) << OP_SHIFT_)
>>    };
>>    
>> -enum StreamId { // Stream ID, (2) [9:8].
>> +enum StreamId : unsigned { // Stream ID, (2) [9:8].
>>      STREAM_ID_DEFAULT_ = 0,
>>      STREAM_ID_LAST_ = 4,
>>      STREAM_ID_FIRST_ = STREAM_ID_DEFAULT_,
>> @@ -308,7 +308,7 @@ enum Id { // HwRegCode, (6) [5:0]
>>      ID_MASK_ = (((1 << ID_WIDTH_) - 1) << ID_SHIFT_)
>>    };
>>    
>> -enum Offset { // Offset, (5) [10:6]
>> +enum Offset : unsigned { // Offset, (5) [10:6]
>>      OFFSET_DEFAULT_ = 0,
>>      OFFSET_SHIFT_ = 6,
>>      OFFSET_WIDTH_ = 5,
>> @@ -318,7 +318,7 @@ enum Offset { // Offset, (5) [10:6]
>>      OFFSET_SRC_PRIVATE_BASE = 0
>>    };
>>    
>> -enum WidthMinusOne { // WidthMinusOne, (5) [15:11]
>> +enum WidthMinusOne : unsigned { // WidthMinusOne, (5) [15:11]
>>      WIDTH_M1_DEFAULT_ = 31,
>>      WIDTH_M1_SHIFT_ = 11,
>>      WIDTH_M1_WIDTH_ = 5,
>> @@ -332,7 +332,7 @@ enum WidthMinusOne { // WidthMinusOne, (
>>    
>>    namespace Swizzle { // Encoding of swizzle macro used in ds_swizzle_b32.
>>    
>> -enum Id { // id of symbolic names
>> +enum Id : unsigned { // id of symbolic names
>>      ID_QUAD_PERM = 0,
>>      ID_BITMASK_PERM,
>>      ID_SWAP,
>> @@ -340,7 +340,7 @@ enum Id { // id of symbolic names
>>      ID_BROADCAST
>>    };
>>    
>> -enum EncBits {
>> +enum EncBits : unsigned {
>>    
>>      // swizzle mode encodings
>>    
>> @@ -372,7 +372,7 @@ enum EncBits {
>>    
>>    namespace SDWA {
>>    
>> -enum SdwaSel {
>> +enum SdwaSel : unsigned {
>>      BYTE_0 = 0,
>>      BYTE_1 = 1,
>>      BYTE_2 = 2,
>> @@ -382,13 +382,13 @@ enum SdwaSel {
>>      DWORD = 6,
>>    };
>>    
>> -enum DstUnused {
>> +enum DstUnused : unsigned {
>>      UNUSED_PAD = 0,
>>      UNUSED_SEXT = 1,
>>      UNUSED_PRESERVE = 2,
>>    };
>>    
>> -enum SDWA9EncValues{
>> +enum SDWA9EncValues : unsigned {
>>      SRC_SGPR_MASK = 0x100,
>>      SRC_VGPR_MASK = 0xFF,
>>      VOPC_DST_VCC_MASK = 0x80,
>> @@ -406,7 +406,7 @@ enum SDWA9EncValues{
>>    
>>    namespace DPP {
>>    
>> -enum DppCtrl {
>> +enum DppCtrl : unsigned {
>>      QUAD_PERM_FIRST   = 0,
>>      QUAD_PERM_LAST    = 0xFF,
>>      DPP_UNUSED1       = 0x100,
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 



More information about the llvm-commits mailing list