[ARM][Thumb2InstrInfo]: fix default '0' opcode when rewriting frame indeces

[DoktorC]

Summary

Hi all,

I noticed that the static functions "positiveOffsetOpcode", "negativeOffsetOpcode" and
"ImmediateOffsetOpcode" (lib/Target/ARM/Thumb2InstrInfo.cpp) may return "0"
(PHI generic instruction) as default opcode.

Briefly. the static method is invoked during re-writing of frame indeces
(Prologue/Epilogue emission phase), and is used to determine the right load/store
instruction to handle the offset. This is done via switch statement in the
"XXXOffsetOpcode" static functions.

However, if none of the cases in the "switch" match, the default
opcode is 0; that is, the generic PHI instruction.

PHI instructions do not make sense in the Prologue/Epilogue emission
phase, and I think it would be more appropriate to default the "switch" to a
"llvm_unreachable" statement.

Best regards,
Lorenzo

[tellenbach]

llvm/lib/Target/ARM/Thumb2InstrInfo.cpp

369

Could you move the default case at the end of the switch statement? This makes it easier to follow the control-flow.

And please remove the exclamation mark !.

399

Same here

429

Same here

[DoktorC]

Moved the 'default's to the end of switches.
Removed the trailing exclamation marks.

Thank you @tellenbach.

[tellenbach]

Thanks for addressing the comments, LGTM!

Do you have commit access? If not I can commit this patch for you. In this case please provide firstname lastname <[e-mail]> in order to attribute the patch correctly.

[DoktorC]

In D69432#1723141, @tellenbach wrote:

Do you have commit access?

No, I do not :(

If not I can commit this patch for you. In this case please provide firstname lastname <[e-mail]> in order to attribute the patch correctly.

Here you are: lorenzo casalino <[e-mail]>

Thank you really much!

llvm/lib/Target/ARM/Thumb2InstrInfo.cpp

while (NumBytes) {
     BaseReg = DestReg;
   }
 }
 
 static unsigned
 negativeOffsetOpcode(unsigned opcode)
 {
   switch (opcode) {
+  default:
+    llvm_unreachable("unknown thumb2 opcode!");
+
   case ARM::t2LDRi12:   return ARM::t2LDRi8;
   case ARM::t2LDRHi12:  return ARM::t2LDRHi8;
   case ARM::t2LDRBi12:  return ARM::t2LDRBi8;
   case ARM::t2LDRSHi12: return ARM::t2LDRSHi8;
   case ARM::t2LDRSBi12: return ARM::t2LDRSBi8;
   case ARM::t2STRi12:   return ARM::t2STRi8;
   case ARM::t2STRBi12:  return ARM::t2STRBi8;
   case ARM::t2STRHi12:  return ARM::t2STRHi8;
   case ARM::t2PLDi12:   return ARM::t2PLDi8;
 
   case ARM::t2LDRi8:
   case ARM::t2LDRHi8:
   case ARM::t2LDRBi8:
   case ARM::t2LDRSHi8:
   case ARM::t2LDRSBi8:
   case ARM::t2STRi8:
   case ARM::t2STRBi8:
   case ARM::t2STRHi8:
   case ARM::t2PLDi8:
     return opcode;
-
-  default:
-    break;
   }
-
-  return 0;
 }
 
 static unsigned
 positiveOffsetOpcode(unsigned opcode)
 {
   switch (opcode) {
+  default:
+    llvm_unreachable("unknown thumb2 opcode!");
+
   case ARM::t2LDRi8:   return ARM::t2LDRi12;
   case ARM::t2LDRHi8:  return ARM::t2LDRHi12;
   case ARM::t2LDRBi8:  return ARM::t2LDRBi12;
   case ARM::t2LDRSHi8: return ARM::t2LDRSHi12;
   case ARM::t2LDRSBi8: return ARM::t2LDRSBi12;
   case ARM::t2STRi8:   return ARM::t2STRi12;
   case ARM::t2STRBi8:  return ARM::t2STRBi12;
   case ARM::t2STRHi8:  return ARM::t2STRHi12;
   case ARM::t2PLDi8:   return ARM::t2PLDi12;
 
   case ARM::t2LDRi12:
   case ARM::t2LDRHi12:
   case ARM::t2LDRBi12:
   case ARM::t2LDRSHi12:
   case ARM::t2LDRSBi12:
   case ARM::t2STRi12:
   case ARM::t2STRBi12:
   case ARM::t2STRHi12:
   case ARM::t2PLDi12:
     return opcode;
-
-  default:
-    break;
   }
-
-  return 0;
 }
 
 static unsigned
 immediateOffsetOpcode(unsigned opcode)
 {
   switch (opcode) {
+  default:
+    llvm_unreachable("unknown thumb2 opcode!");
+
   case ARM::t2LDRs:   return ARM::t2LDRi12;
   case ARM::t2LDRHs:  return ARM::t2LDRHi12;
   case ARM::t2LDRBs:  return ARM::t2LDRBi12;
   case ARM::t2LDRSHs: return ARM::t2LDRSHi12;
   case ARM::t2LDRSBs: return ARM::t2LDRSBi12;
   case ARM::t2STRs:   return ARM::t2STRi12;
   case ARM::t2STRBs:  return ARM::t2STRBi12;
   case ARM::t2STRHs:  return ARM::t2STRHi12;
immediateOffsetOpcode(unsigned opcode)
   case ARM::t2LDRBi8:
   case ARM::t2LDRSHi8:
   case ARM::t2LDRSBi8:
   case ARM::t2STRi8:
   case ARM::t2STRBi8:
   case ARM::t2STRHi8:
   case ARM::t2PLDi8:
     return opcode;
-
-  default:
-    break;
   }
-
-  return 0;
 }
 
 bool llvm::rewriteT2FrameIndex(MachineInstr &MI, unsigned FrameRegIdx,
                                unsigned FrameReg, int &Offset,
                                const ARMBaseInstrInfo &TII,
                                const TargetRegisterInfo *TRI) {
   unsigned Opcode = MI.getOpcode();
   const MCInstrDesc &Desc = MI.getDesc();