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

[Lorenzo Casalino via Phabricator via llvm-commits]

DoktorC created this revision.
DoktorC added a reviewer: t.p.northover.
Herald added subscribers: llvm-commits, hiraditya, kristof.beyls.
Herald added a project: LLVM.
DoktorC edited the summary of this revision.

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


Repository:
  rL LLVM

https://reviews.llvm.org/D69432

Files:
  llvm/lib/Target/ARM/Thumb2InstrInfo.cpp


Index: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
===================================================================
--- llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
+++ llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
@@ -366,6 +366,9 @@
 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;
@@ -386,18 +389,16 @@
   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;
@@ -418,18 +419,16 @@
   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;
@@ -459,12 +458,7 @@
   case ARM::t2STRHi8:
   case ARM::t2PLDi8:
     return opcode;
-
-  default:
-    break;
   }
-
-  return 0;
 }
 
 bool llvm::rewriteT2FrameIndex(MachineInstr &MI, unsigned FrameRegIdx,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69432.226418.patch
Type: text/x-patch
Size: 1540 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191025/f9d9a9a6/attachment.bin>