Subzero: Remove Variable::NeedsStackSlot.

Subzero: Remove Variable::NeedsStackSlot.

Instead, separately compute it during prolog generation via another
pass over the Cfg.

This may slow down translation by ~1%, but it greatly simplifies the
management of this flag/property.

The higher motivation is to pull this management out of register
allocation to make it easier to extend register allocation for other
uses.

BUG= none
R=jvoung@chromium.org

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

[Jim Stichnoth, 2014-11-11 16:51:14]

stichnot@chromium.org changed reviewers:
+ jvoung@chromium.org

[jvoung (off chromium), 2014-11-11 17:18:10]

https://codereview.chromium.org/692633004/diff/20001/src/IceTargetLoweringX86...
File src/IceTargetLoweringX8632.cpp (right):

https://codereview.chromium.org/692633004/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:676: continue;
If the instruction isRedundantAssign() should this be skipped too?

https://codereview.chromium.org/692633004/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:678: NeedsStackSlot[Var->getIndex()] = true;
should this check if (!Var->hasReg()) first?

[Jim Stichnoth, 2014-11-11 17:43:39]

https://codereview.chromium.org/692633004/diff/20001/src/IceTargetLoweringX86...
File src/IceTargetLoweringX8632.cpp (right):

https://codereview.chromium.org/692633004/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:676: continue;
On 2014/11/11 17:18:09, jvoung wrote:
> If the instruction isRedundantAssign() should this be skipped too?

No, isRedundantAssign() depends on the frame offsets being assigned, and at this
point that hasn't happened yet.

https://codereview.chromium.org/692633004/diff/20001/src/IceTargetLoweringX86...
src/IceTargetLoweringX8632.cpp:678: NeedsStackSlot[Var->getIndex()] = true;
On 2014/11/11 17:18:10, jvoung wrote:
> should this check if (!Var->hasReg()) first?

Good point.  It turns out that neither hasReg() check is actually necessary,
because the check is repeated (in line 741 of this version of the file).

I removed the check here, and renamed NeedsStackSlot to IsVarReferenced to
capture a more general meaning.

[jvoung (off chromium), 2014-11-11 17:48:20]

lgtm

[Jim Stichnoth, 2014-11-11 17:49:09]

Committed patchset #3 (id:40001) manually as
33c8064167a1623f2d5c5738302193ee8e10411b (presubmit successful).