UnimplementedLoweringError's message now includes the instruction name.

src/IceInst.cpp

 //===- subzero/src/IceInst.cpp - High-level instruction implementation ----===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 59 matching lines @@
     ICEINSTICMP_TABLE
 #undef X
 };
 
 } // end of anonymous namespace
 
 Inst::Inst(Cfg *Func, InstKind Kind, SizeT MaxSrcs, Variable *Dest)
     : Kind(Kind), Number(Func->newInstNumber()), Dest(Dest), MaxSrcs(MaxSrcs),
       Srcs(Func->allocateArrayOf<Operand *>(MaxSrcs)), LiveRangesEnded(0) {}
 
+IceString Inst::getInstName() const {
+  if (!BuildDefs::dump())
+    return "???";
+
+  switch (Kind) {
+#define X(InstrKind, name)                                                     \

[Jim Stichnoth, 2016/01/27 22:31:51]

If you want to take the X() approach, I would add a new ICEINST_TABLE x-macro to
IceInst.def, use it here, and also use it in IceInst.h for the definition of
enum InstKind.

+  case InstrKind:                                                              \
+    return name

[John, 2016/01/27 21:14:46]

You could have just returned a capitalized version of the Inst (using the
stringfier #), but this is fine.

[Eric Holk, 2016/01/27 21:28:10]

Jim wanted them to be lowercase for use in dump(), but that may have just
applied to arithmetic instructions.

+    X(Unreachable, "unreachable");
+    X(Alloca, "alloca");
+    X(Arithmetic, "arithmetic");
+    X(Br, "br");
+    X(Call, "call");
+    X(Cast, "cast");
+    X(ExtractElement, "extractelement");
+    X(Fcmp, "fcmp");
+    X(Icmp, "icmp");
+    X(IntrinsicCall, "intrinsiccall");
+    X(InsertElement, "insertelement");
+    X(Load, "load");
+    X(Phi, "phi");
+    X(Ret, "ret");
+    X(Select, "select");
+    X(Store, "store");
+    X(Switch, "switch");
+    X(Assign, "assign");
+    X(BundleLock, "bundlelock");
+    X(BundleUnlock, "bundleunlock");
+    X(FakeDef, "fakedef");
+    X(FakeUse, "fakeuse");
+    X(FakeKill, "fakekill");
+    X(JumpTable, "jumptable");
+#undef X
+  default:
+    assert(Kind >= Target);
+    return "target";
+  }
+}
+
 // Assign the instruction a new number.
 void Inst::renumber(Cfg *Func) {
   Number = isDeleted() ? NumberDeleted : Func->newInstNumber();
 }
 
 // Delete the instruction if its tentative Dead flag is still set after
 // liveness analysis.
 void Inst::deleteIfDead() {
   if (Dead)
     setDeleted();
@@ skipping 136 matching lines @@
   addSource(ByteCount);
 }
 
 InstArithmetic::InstArithmetic(Cfg *Func, OpKind Op, Variable *Dest,
                                Operand *Source1, Operand *Source2)
     : InstHighLevel(Func, Inst::Arithmetic, 2, Dest), Op(Op) {
   addSource(Source1);
   addSource(Source2);
 }
 
+IceString InstArithmetic::getInstName() const {
+  if (!BuildDefs::dump())
+    return "???";
+
+  return InstArithmeticAttributes[getOp()].DisplayString;
+}
+
 const char *InstArithmetic::getOpName(OpKind Op) {
   size_t OpIndex = static_cast<size_t>(Op);
   return OpIndex < InstArithmetic::_num
              ? InstArithmeticAttributes[OpIndex].DisplayString
              : "???";
 }
 
 bool InstArithmetic::isCommutative() const {
   return InstArithmeticAttributes[getOp()].IsCommutative;
 }
@@ skipping 305 matching lines @@
   dump(Func);
   dumpExtras(Func);
   Str << "\n";
 }
 
 void Inst::dump(const Cfg *Func) const {
   if (!BuildDefs::dump())
     return;
   Ostream &Str = Func->getContext()->getStrDump();
   dumpDest(Func);
-  Str << " =~ ";
+  Str << " =~ " << getInstName() << " ";
   dumpSources(Func);
 }
 
 void Inst::dumpExtras(const Cfg *Func) const {
   if (!BuildDefs::dump())
     return;
   Ostream &Str = Func->getContext()->getStrDump();
   bool First = true;
   // Print "LIVEEND={a,b,c}" for all source operands whose live ranges are
   // known to end at this instruction.
@@ skipping 40 matching lines @@
     return;
   if (getDest())
     getDest()->dump(Func);
 }
 
 void InstAlloca::dump(const Cfg *Func) const {
   if (!BuildDefs::dump())
     return;
   Ostream &Str = Func->getContext()->getStrDump();
   dumpDest(Func);
   Str << " = alloca i8, i32 ";

[Jim Stichnoth, 2016/01/27 22:31:51]

I was actually thinking this should be changed to:

  Str << " = " << getInstName() << " i8, i32 ";

and similar for the other instructions wherever a hard-coded instruction name is
used, much like what you did for InstArithmetic.

The idea is that exercising getInstName() in the dump() methods gives it a
fighting chance of actually being tested and not rotting.

   getSizeInBytes()->dump(Func);
   if (getAlignInBytes())
     Str << ", align " << getAlignInBytes();
 }
 
 void InstArithmetic::dump(const Cfg *Func) const {
   if (!BuildDefs::dump())
     return;
   Ostream &Str = Func->getContext()->getStrDump();
   dumpDest(Func);
-  Str << " = " << InstArithmeticAttributes[getOp()].DisplayString << " "
-      << getDest()->getType() << " ";
+  Str << " = " << getInstName() << " " << getDest()->getType() << " ";
   dumpSources(Func);
 }
 
 void InstAssign::dump(const Cfg *Func) const {
   if (!BuildDefs::dump())
     return;
   Ostream &Str = Func->getContext()->getStrDump();
   dumpDest(Func);
   Str << " = " << getDest()->getType() << " ";
   dumpSources(Func);
@@ skipping 322 matching lines @@
     // upper 32 bits of rax. We need to recognize and preserve these.
     return true;
   }
   if (!Dest->hasReg() && !SrcVar->hasReg() &&
       Dest->getStackOffset() == SrcVar->getStackOffset())
     return true;
   return false;
 }
 
 } // end of namespace Ice