Subzero: Fix build errors with LLVM trunk.

Subzero: Fix build errors with LLVM trunk.

1. Some explicit conversions between Inst* and InstList::iterator are needed.

2. llvm::sys::PrintStackTraceOnErrorSignal() has a new argument.

3. A previous CL neglected to plumb in SUBZERO_REVISION for the cmake build.

BUG= none
R=eholk@chromium.org

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

[Jim Stichnoth, 2016-08-16 20:58:30]

Description was changed from

==========
Subzero: Fix build errors with LLVM trunk.

BUG= none
==========

to

==========
Subzero: Fix build errors with LLVM trunk.

1. Some explicit conversions between Inst* and InstList::iterator are needed.

2. llvm::sys::PrintStackTraceOnErrorSignal() has a new argument.

3. A previous CL neglected to plumb in SUBZERO_REVISION for the cmake build.

BUG= none
==========

[Jim Stichnoth, 2016-08-16 20:59:42]

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

[Jim Stichnoth, 2016-08-16 21:00:40]

+dschuff for the cmake stuff.

[Eric Holk, 2016-08-17 00:33:46]

lgtm

https://codereview.chromium.org/2247253005/diff/1/src/IceTargetLoweringX86Bas...
File src/IceTargetLoweringX86BaseImpl.h (right):

https://codereview.chromium.org/2247253005/diff/1/src/IceTargetLoweringX86Bas...
src/IceTargetLoweringX86BaseImpl.h:812: Inst *CurInst =
iteratorToInst(Context.getCur());
Would just `*Context.getCur()` work?

[Jim Stichnoth, 2016-08-17 04:43:30]

https://codereview.chromium.org/2247253005/diff/1/src/IceTargetLoweringX86Bas...
File src/IceTargetLoweringX86BaseImpl.h (right):

https://codereview.chromium.org/2247253005/diff/1/src/IceTargetLoweringX86Bas...
src/IceTargetLoweringX86BaseImpl.h:812: Inst *CurInst =
iteratorToInst(Context.getCur());
On 2016/08/17 00:33:46, Eric Holk wrote:
> Would just `*Context.getCur()` work?

Given that the original code was:
  Inst *CurInst = Context.getCur();
I wouldn't expect adding a '*' to work:
  Inst *CurInst = *Context.getCur();

This does work though:
  Inst *CurInst = &*Context.getCur();
but it's really ugly and is in fact how iteratorToInst() is defined in
IceInst.h.

[Derek Schuff, 2016-08-17 06:41:23]

On 2016/08/17 04:43:30, stichnot wrote:
>
https://codereview.chromium.org/2247253005/diff/1/src/IceTargetLoweringX86Bas...
> File src/IceTargetLoweringX86BaseImpl.h (right):
> 
>
https://codereview.chromium.org/2247253005/diff/1/src/IceTargetLoweringX86Bas...
> src/IceTargetLoweringX86BaseImpl.h:812: Inst *CurInst =
> iteratorToInst(Context.getCur());
> On 2016/08/17 00:33:46, Eric Holk wrote:
> > Would just `*Context.getCur()` work?
> 
> Given that the original code was:
>   Inst *CurInst = Context.getCur();
> I wouldn't expect adding a '*' to work:
>   Inst *CurInst = *Context.getCur();
> 
> This does work though:
>   Inst *CurInst = &*Context.getCur();
> but it's really ugly and is in fact how iteratorToInst() is defined in
> IceInst.h.

FWIW LLVM is full of &* and similar because there's no consistency about which
kinds of things are pointers and which kinds of things are references.

[Jim Stichnoth, 2016-08-17 15:53:11]

On 2016/08/17 06:41:23, Derek Schuff wrote:
> On 2016/08/17 04:43:30, stichnot wrote:
> >
>
https://codereview.chromium.org/2247253005/diff/1/src/IceTargetLoweringX86Bas...
> > File src/IceTargetLoweringX86BaseImpl.h (right):
> > 
> >
>
https://codereview.chromium.org/2247253005/diff/1/src/IceTargetLoweringX86Bas...
> > src/IceTargetLoweringX86BaseImpl.h:812: Inst *CurInst =
> > iteratorToInst(Context.getCur());
> > On 2016/08/17 00:33:46, Eric Holk wrote:
> > > Would just `*Context.getCur()` work?
> > 
> > Given that the original code was:
> >   Inst *CurInst = Context.getCur();
> > I wouldn't expect adding a '*' to work:
> >   Inst *CurInst = *Context.getCur();
> > 
> > This does work though:
> >   Inst *CurInst = &*Context.getCur();
> > but it's really ugly and is in fact how iteratorToInst() is defined in
> > IceInst.h.
> 
> FWIW LLVM is full of &* and similar because there's no consistency about which
> kinds of things are pointers and which kinds of things are references.

My memory is a bit fuzzy from when I first looked into this, but you're right
that the code base has a lot of &* for converting an ilist<Foo>::iterator into a
Foo pointer.  I greatly prefer the clarity of iteratorToFoo().

Fortunately, LLVM added the getIterator() method to the ilist<> internals, which
also adds clarity.

[Jim Stichnoth, 2016-08-17 16:12:55]

Description was changed from

==========
Subzero: Fix build errors with LLVM trunk.

1. Some explicit conversions between Inst* and InstList::iterator are needed.

2. llvm::sys::PrintStackTraceOnErrorSignal() has a new argument.

3. A previous CL neglected to plumb in SUBZERO_REVISION for the cmake build.

BUG= none
==========

to

==========
Subzero: Fix build errors with LLVM trunk.

1. Some explicit conversions between Inst* and InstList::iterator are needed.

2. llvm::sys::PrintStackTraceOnErrorSignal() has a new argument.

3. A previous CL neglected to plumb in SUBZERO_REVISION for the cmake build.

BUG= none
R=eholk@chromium.org

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

[Jim Stichnoth, 2016-08-17 16:12:56]

Committed patchset #1 (id:1) manually as
efdf412032f7622a0663696896708d75b82e92f6 (presubmit successful).