[AMDGPU] Fix warning

[arsenm]

Where are you seeing this warning? It's switching over all values of the enum so I see the warning for that if I add the default

lib/Target/AMDGPU/AMDGPUInstrInfo.cpp

147–149

Fall through comment doesn't make sense after unreachable

[kzhuravl]

I am getting this warning when building without default:

/home/kzhuravl/Lightning/llvm/lib/Target/AMDGPU/AMDGPUInstrInfo.cpp: In function â?~SIEncodingFamily subtargetEncodingFamily(const llvm::AMDGPUSubtarget&)â?T:
/home/kzhuravl/Lightning/llvm/lib/Target/AMDGPU/AMDGPUInstrInfo.cpp:160:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^

I am not getting a warning with default, which makes sense to me.

Underlying type of an unscoped enum is some integral type (int, unsigned int, etc... implementation-defined). So, I do not think, that switching over all defined enum values is enough:

• What happens if, for some reason, ST.getGeneration() returns an undefined enum value (for example, some garbage)? - undefined behavior.
• What happens if we add a new enum value, and forget to add it to a switch statement? - undefined behavior.

Someone can argue that above 2 cases can never happen, hence it is a warning, not a compilation error. But I'd rather know that unreachable will be executed if something goes wrong.

Please, correct me if I am wrong.

I am not sure why you experience the reverse behavior. Do you have any other modified files? Can you paste the warning you are getting?

[arsenm]

It's undefined to be out of bounds value. Clang warns when there is a default when all enum values are covered. It won't warn on the non void here because it won't reach there. I think you have to move the unreachable after the switch and remove default

[kzhuravl]

Ah, you are right (http://llvm.org/docs/CodingStandards.html#don-t-use-default-labels-in-fully-covered-switches-over-enumerations)

thanks

[kzhuravl]

Address review comments

[arsenm]

LGTM

[kzhuravl]

rL273872

lib/Target/AMDGPU/AMDGPUInstrInfo.cpp

 static int getMCOpcode(uint16_t Opcode, unsigned Gen) {
   return getMCOpcodeGen(Opcode, static_cast<Subtarget>(Gen));
 }
 }
 }
 
 static SIEncodingFamily subtargetEncodingFamily(const AMDGPUSubtarget &ST) {
   switch (ST.getGeneration()) {
   case AMDGPUSubtarget::SOUTHERN_ISLANDS:
   case AMDGPUSubtarget::SEA_ISLANDS:
     return SIEncodingFamily::SI;
   case AMDGPUSubtarget::VOLCANIC_ISLANDS:
     return SIEncodingFamily::VI;
 
   // FIXME: This should never be called for r600 GPUs.
   case AMDGPUSubtarget::R600:
   case AMDGPUSubtarget::R700:
   case AMDGPUSubtarget::EVERGREEN:
   case AMDGPUSubtarget::NORTHERN_ISLANDS:
     return SIEncodingFamily::SI;
   }
+  llvm_unreachable("unexpected generation");
 }
 
 int AMDGPUInstrInfo::pseudoToMCOpcode(int Opcode) const {
   int MCOp = AMDGPU::getMCOpcode(Opcode, subtargetEncodingFamily(ST));
 
   // -1 means that Opcode is already a native instruction.
   if (MCOp == -1)
     return Opcode;
 
   // (uint16_t)-1 means that Opcode is a pseudo instruction that has
   // no encoding in the given subtarget generation.
   if (MCOp == (uint16_t)-1)
     return -1;
 
   return MCOp;
 }