Restore function-local variables to use a vector.

Restore function-local variables to use a vector.

CL 1282523002 changed the bitcode parser from using a vector, to using
an unordered map. This was done because one could forward reference a
local variable, and would freeze the computer trying to allocate a
vector large enough to contain the index.

This patch goes back to using vectors. To fix the forward variable
reference, we use the number of bytes in the function to determine if
the index is possible. This stops very large (probematic) vector
resizes from happening.

BUG=None
R=stichnot@chromium.org

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

[Karl, 2015-08-14 22:07:17]

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

[Karl, 2015-08-14 22:07:18]

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

[Karl, 2015-08-17 17:44:04]

ping?

[Karl, 2015-08-17 17:47:23]

On 2015/08/17 17:44:04, Karl wrote:
> ping?

Sorry, Ignore this ping. I still want the code reviewed, but it was my fault
that it didn't get sent out till a few minutes ago.

[Jim Stichnoth, 2015-08-17 19:41:03]

lgtm

https://codereview.chromium.org/1293923002/diff/20001/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/1293923002/diff/20001/src/PNaClTranslator.cpp...
src/PNaClTranslator.cpp:1339: std::vector<Ice::Operand *> LocalOperands;
It occurs to me that using STL containers with the default allocator may be
partially responsible for Subzero's scalability problems (along with similar
uses outside the parser code).

If containers are local to a Cfg, then a simple change is e.g. from:
  std::vector<Foo>
to:
  std::vector<Foo, CfgLocalAllocator<Foo>>
The same holds for list<>, unordered_map<>, etc.

It would be good to try these changes in a separate CL.  I would look at
-threads=0 and -threads=1 performance, before and after the change.

The same would be done later to the non-parser code as well.

[Karl, 2015-08-17 19:43:34]

Committed patchset #2 (id:20001) manually as
7a99327dae74a3abd7f222de1b218d20da665e1c (presubmit successful).

[Karl, 2015-08-17 19:47:34]

https://codereview.chromium.org/1293923002/diff/20001/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/1293923002/diff/20001/src/PNaClTranslator.cpp...
src/PNaClTranslator.cpp:1339: std::vector<Ice::Operand *> LocalOperands;
On 2015/08/17 19:41:03, stichnot wrote:
> It occurs to me that using STL containers with the default allocator may be
> partially responsible for Subzero's scalability problems (along with similar
> uses outside the parser code).
> 
> If containers are local to a Cfg, then a simple change is e.g. from:
>   std::vector<Foo>
> to:
>   std::vector<Foo, CfgLocalAllocator<Foo>>
> The same holds for list<>, unordered_map<>, etc.
> 
> It would be good to try these changes in a separate CL.  I would look at
> -threads=0 and -threads=1 performance, before and after the change.
> 
> The same would be done later to the non-parser code as well.

Acknowledged.