UnimplementedLoweringError's message now includes the instruction name.

UnimplementedLoweringError's message now includes the instruction name.

R=jpp@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=e37076a176e04eee0f35058ccd2109754c193015

[Eric Holk, 2016-01-27 20:41:24]

eholk@chromium.org changed reviewers:
+ jpp@chromium.org, kschimpf@google.com, sehr@chromium.org,
stichnot@chromium.org

[Eric Holk, 2016-01-27 20:41:25]

This has the changes I pulled out of the vector subtraction patch.

Jim, can you pay special attention to the dump methods? Is this what you had in
mind? It looks like there might be a way to consolidate more of these, but I
wasn't sure the best way to do it.

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

lgtm

https://codereview.chromium.org/1639063002/diff/80001/src/IceInst.cpp
File src/IceInst.cpp (right):

https://codereview.chromium.org/1639063002/diff/80001/src/IceInst.cpp#newcode87
src/IceInst.cpp:87: return name
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]

https://codereview.chromium.org/1639063002/diff/80001/src/IceInst.cpp
File src/IceInst.cpp (right):

https://codereview.chromium.org/1639063002/diff/80001/src/IceInst.cpp#newcode87
src/IceInst.cpp:87: return name
On 2016/01/27 21:14:46, John wrote:
> You could have just returned a capitalized version of the Inst (using the
> stringfier #), but this is fine.

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

[Eric Holk, 2016-01-27 22:06:38]

Description was changed from

==========
UnimplementedLoweringError's message now includes the instruction name.
==========

to

==========
UnimplementedLoweringError's message now includes the instruction name.

R=jpp@chromium.org

Committed:
https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a...
==========

[Eric Holk, 2016-01-27 22:06:39]

Committed patchset #5 (id:80001) manually as
e37076a176e04eee0f35058ccd2109754c193015 (presubmit successful).

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

https://codereview.chromium.org/1639063002/diff/80001/src/IceInst.cpp
File src/IceInst.cpp (right):

https://codereview.chromium.org/1639063002/diff/80001/src/IceInst.cpp#newcode85
src/IceInst.cpp:85: #define X(InstrKind, name)                                  
                  \
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.

https://codereview.chromium.org/1639063002/diff/80001/src/IceInst.cpp#newcode668
src/IceInst.cpp:668: Str << " = alloca i8, i32 ";
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.

https://codereview.chromium.org/1639063002/diff/80001/src/IceTargetLowering.h
File src/IceTargetLowering.h (right):

https://codereview.chromium.org/1639063002/diff/80001/src/IceTargetLowering.h...
src/IceTargetLowering.h:59: ("Not yet implemented: " +
Instr->getInstName()).c_str());           \
The extra level of parentheses is odd, i.e.
  llvm_unreachable(("foo"));