Subzero. ARM32. Vector lowering. Subtract.

src/IceInst.h

Index: src/IceInst.h
diff --git a/src/IceInst.h b/src/IceInst.h
index 85fde52b316fdb67d4f23481a277f92997673838..085b04573df0b56724c7688e8a148024da231fe0 100644
--- a/src/IceInst.h
+++ b/src/IceInst.h
@@ -76,6 +76,60 @@ public:
   };
   static_assert(Target <= Target_Max, "Must not be above max.");
   InstKind getKind() const { return Kind; }
+  IceString getInstName() const {

[John, 2016/01/26 19:07:17]

Can you move the method to IceInst.cpp?

[John, 2016/01/26 19:07:17]

this is really useful, but does not belong in this patch. can you move it to its
own cl?

This is also going to increase what's called minimal mode (make MINIMAL=1),
which builds the binary shipped to clients. I would suggest you to add a guard

  if (!BuildDefs::dump())
    return "";

I am not sure getInstName() conveys the purpose of this method. It returns the
instruction **type**, but its name, converted to a string, but I can't think of
anything better.

[Jim Stichnoth, 2016/01/26 19:13:10]

Could you also change the dump() methods in IceInst.cpp to use this method
instead of hard-coded strings?  This would also mean lower-casing the returned
strings in this method.

[Jim Stichnoth, 2016/01/26 19:13:10]

Add the following to the beginning of this method:

if (!BuildDefs::dump())
  return "???";

or something to that effect.  This reduces the translator size in a MINIMAL
build.

[Eric Holk, 2016/01/26 22:49:24]

Sure.

[Eric Holk, 2016/01/26 22:49:24]

Yup, I'm working on that now.
Sure.
Yeah, I don't really like the name either. What about getKindAsString()? Or
maybe even just toString()?

[Eric Holk, 2016/01/26 22:49:24]

I made some of the obvious changes, but it looks like more generally we might
just be able to get rid of most of the special cases of dump.

+    switch (Kind) {
+    case Unreachable:

[John, 2016/01/26 19:07:17]

I would suggest the following:

switch (Kind) {
#define CASE(InstKind) case InstKind: return #InstKind;
  CASE(Unreachable)
  CASE(Alloca)
  CASE(Arithmetic)
  ...
  CASE(JumpTable)
  default:
  CASE(Target)
#undef X
}

the default is very important

+      return "Unreachable";
+    case Alloca:
+      return "Alloca";
+    case Arithmetic:
+      return "Arithmetic";
+    case Br:
+      return "Br";
+    case Call:
+      return "Call";
+    case Cast:
+      return "Cast";
+    case ExtractElement:
+      return "ExtractElement";
+    case Fcmp:
+      return "Fcmp";
+    case Icmp:
+      return "Icmp";
+    case IntrinsicCall:
+      return "IntrinsicCall";
+    case InsertElement:
+      return "InsertElement";
+    case Load:
+      return "Load";
+    case Phi:
+      return "Phi";
+    case Ret:
+      return "Ret";
+    case Select:
+      return "Select";
+    case Store:
+      return "Store";
+    case Switch:
+      return "Switch";
+    case Assign:
+      return "Assign";
+    case BundleLock:
+      return "BundleLock";
+    case BundleUnlock:
+      return "BundleUnlock";
+    case FakeDef:
+      return "FakeDef";
+    case FakeUse:
+      return "FakeUse";
+    case FakeKill:
+      return "FakeKill";
+    case JumpTable:
+      return "JumpTable";
+    default:
+      return "Target";

[Jim Stichnoth, 2016/01/26 19:13:10]

Add an assert:
  assert(Kind >= Target);
just to make sure that newly added high-level instruction kinds don't forget to
add their string here.

[Eric Holk, 2016/01/26 22:49:24]

Okay.

+    }
+  }
 
   InstNumberT getNumber() const { return Number; }
   void renumber(Cfg *Func);
@@ -288,6 +342,19 @@ public:
         InstArithmetic(Func, Op, Dest, Source1, Source2);
   }
   OpKind getOp() const { return Op; }
+
+  IceString getInstName() const {

[John, 2016/01/26 19:07:17]

add the

if (!BuildDefs::dump()) return "";

and move this to the .cpp file.

[Jim Stichnoth, 2016/01/26 19:13:10]

Are you shadowing a nonvirtual method name in a base class?  That seems a little
dangerous or error-prone.  Should this be virtual, with override or final here?

[Eric Holk, 2016/01/26 22:49:24]

Okay.

[Eric Holk, 2016/01/26 22:49:24]

I was thinking the same thing. I made it virtual.

+    switch (Op) {
+#define X(tag, str, commutative)                                               \
+  case tag:                                                                    \
+    return str;
+      ICEINSTARITHMETIC_TABLE
+#undef X
+    default:
+      return "Unknown";
+    }
+  }
+
   static const char *getOpName(OpKind Op);
   bool isCommutative() const;
   void dump(const Cfg *Func) const override;