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

Mekhanoshin, Stanislav via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 13 13:55:42 PDT 2019


Thank you: https://reviews.llvm.org/D59330

Stas

-----Original Message-----
From: Mikael Holmén <mikael.holmen at ericsson.com> 
Sent: 13 марта 2019 г. 3:39
To: Mekhanoshin, Stanislav <Stanislav.Mekhanoshin at amd.com>; llvm-commits at lists.llvm.org
Subject: Re: [llvm] r355836 - [AMDGPU] Mark enum types in SIDefines.h as unsigned

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/AsmPa
> rser/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/Disas
> sembler/AMDGPUDisassembler.cpp?rev=355836&r1=355835&r2=355836&view=dif
> f 
> ======================================================================
> ========
> --- 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/InstP
> rinter/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/SIDef
> ines.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
> 


More information about the llvm-commits mailing list