Fix bug when atomic load is fused with an arith op (and not in the entry BB)

Fix bug when atomic load is fused with an arith op (and not in the entry BB)

Normally, the FakeUse for preserving the atomic load ends
up on the load's Dest. However, for fused load+add, the load
is deleted, and its Dest is no longer defined. This trips
up the liveness analysis when it happens on a non-entry
block. So the FakeUse should be for the add's dest instead,
in that case.

We have no access to the add, so introduce a
getLastInserted() helper. A couple of ways to do that:
- modify insert() to track explicitly
- rewind from Next one step

Either that, or we disable the fusing for atomic loads.

BUG=  https://code.google.com/p/nativeclient/issues/detail?id=3882
R=stichnot@chromium.org

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

[Jim Stichnoth, 2014-07-29 17:54:09]

I think that this issue would be fixed more robustly by modifying the
instruction fusing code in lowerLoad() to set NewArith->HasSideEffects as the
union of the input instructions' HasSideEffects fields.  Then the FakeUse
instructions wouldn't be needed.

However, I'm afraid this would be the first step down the rabbit hole of
maintaining HasSideEffects across all lowering code.  For example,
lowerArithmetic() would need to do extra work.

As such, I would say LGTM with nits, and we can consider the greater change in a
separate CL.

https://codereview.chromium.org/417353003/diff/20001/src/IceTargetLowering.cpp
File src/IceTargetLowering.cpp (right):

https://codereview.chromium.org/417353003/diff/20001/src/IceTargetLowering.cp...
src/IceTargetLowering.cpp:56: --I;
do {
  --I;
} while (I != Begin && (*I)->isDeleted());

https://codereview.chromium.org/417353003/diff/20001/src/IceTargetLowering.h
File src/IceTargetLowering.h (right):

https://codereview.chromium.org/417353003/diff/20001/src/IceTargetLowering.h#...
src/IceTargetLowering.h:69: // Begin is a copy of Insts.begin(), used if
iterators need to be rewinded.
rewound

https://codereview.chromium.org/417353003/diff/20001/src/IceTargetLowering.h#...
src/IceTargetLowering.h:76: void rewind(InstList::iterator &I) const;
I assumed on first read that "rewind" meant "rewind back to the beginning".  Is
there a clearer name?  The best I can think of so far is advanceForward() and
advanceBackward(), or advanceNext() and advancePrev().

[jvoung (off chromium), 2014-07-30 15:17:14]

https://codereview.chromium.org/417353003/diff/20001/src/IceTargetLowering.cpp
File src/IceTargetLowering.cpp (right):

https://codereview.chromium.org/417353003/diff/20001/src/IceTargetLowering.cp...
src/IceTargetLowering.cpp:56: --I;
On 2014/07/29 17:54:09, stichnot wrote:
> do {
>   --I;
> } while (I != Begin && (*I)->isDeleted());

Done.

https://codereview.chromium.org/417353003/diff/20001/src/IceTargetLowering.h
File src/IceTargetLowering.h (right):

https://codereview.chromium.org/417353003/diff/20001/src/IceTargetLowering.h#...
src/IceTargetLowering.h:69: // Begin is a copy of Insts.begin(), used if
iterators need to be rewinded.
On 2014/07/29 17:54:09, stichnot wrote:
> rewound

Done, used the "backward" language to match the new method name.

https://codereview.chromium.org/417353003/diff/20001/src/IceTargetLowering.h#...
src/IceTargetLowering.h:76: void rewind(InstList::iterator &I) const;
On 2014/07/29 17:54:09, stichnot wrote:
> I assumed on first read that "rewind" meant "rewind back to the beginning". 
Is
> there a clearer name?  The best I can think of so far is advanceForward() and
> advanceBackward(), or advanceNext() and advancePrev().

Good point, hmm... going to go with advanceForward() and advanceBackward().
There's already a method called "advanceNext()" for moving the "Next" iterator,
so I didn't go with that.

There's also the shorter, "next()" and "previous()" naming scheme, but "next()"
might be a confusing name alongside the Next field, unless Next is renamed to
InsertPoint.

[jvoung (off chromium), 2014-07-30 17:06:09]

Committed patchset #4 manually as re6e497d (presubmit successful).