PNaCl Dynamic Linking: Storing dependencies from command line in PLL.

PNaCl Dynamic Linking: Storing dependencies from command line in PLL.

Used by https://codereview.chromium.org/1825893002/

TEST=run_pll_hello_world_test
BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4351
R=mseaborn@chromium.org

Committed: https://chromium.googlesource.com/native_client/pnacl-llvm/+/3a2a7d006b87fc9ff029503b1f68d5f4428db4c7

[Sean Klein, 2016-03-23 17:35:43]

Description was changed from

==========
PNaCl Dynamic Linking: Storing dependencies from command line in PLL.

TEST=run_pll_hello_world_test
BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4351
==========

to

==========
PNaCl Dynamic Linking: Storing dependencies from command line in PLL.

Used by https://codereview.chromium.org/1825893002/

TEST=run_pll_hello_world_test
BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4351
==========

[Sean Klein, 2016-03-28 21:09:40]

smklein@chromium.org changed reviewers:
+ mseaborn@chromium.org

[Mark Seaborn, 2016-03-29 15:29:39]

LGTM.  Can you change the convert-to-pso.ll test to exercise this option (even
if it doesn't check that it affects the output)?

https://codereview.chromium.org/1829793002/diff/1/lib/Transforms/NaCl/Convert...
File lib/Transforms/NaCl/ConvertToPSO.cpp (right):

https://codereview.chromium.org/1829793002/diff/1/lib/Transforms/NaCl/Convert...
lib/Transforms/NaCl/ConvertToPSO.cpp:67: LibraryDependencies("deps",
Remember that this is an option for the 'opt' tool.  "-deps" is a bit too
generic.  How about "-convert-to-pso-deps"?

https://codereview.chromium.org/1829793002/diff/1/lib/Transforms/NaCl/Convert...
lib/Transforms/NaCl/ConvertToPSO.cpp:663: for (unsigned i = 0; i !=
LibraryDependencies.size(); ++i) {
You can use "for (... : LibrariesDependencies)"

[Mark Seaborn, 2016-03-29 15:40:06]

https://codereview.chromium.org/1829793002/diff/1/lib/Transforms/NaCl/Convert...
File lib/Transforms/NaCl/ConvertToPSO.cpp (right):

https://codereview.chromium.org/1829793002/diff/1/lib/Transforms/NaCl/Convert...
lib/Transforms/NaCl/ConvertToPSO.cpp:699: // Dependencies List
When you rebase, this should go at the end, after the TLS template fields, so as
not to break when rolled in.

[Mark Seaborn, 2016-03-29 15:53:21]

Can you add a test for this format in pll_symbols_test.cc, please?  (In a
separate change from https://codereview.chromium.org/1825893002/ for quicker
review.)

I'd like to make sure that we have tests for the format of the PLL data that are
separate from tests & implementation of how the PLL data is handled.

[Sean Klein, 2016-03-29 17:58:29]

Test added to pll_symbols_test here: https://codereview.chromium.org/1841113002/

I had to pull in a small number of changes from the larger,
dependency plumbing CL, but I kept it as minimal as I could.

https://codereview.chromium.org/1829793002/diff/1/lib/Transforms/NaCl/Convert...
File lib/Transforms/NaCl/ConvertToPSO.cpp (right):

https://codereview.chromium.org/1829793002/diff/1/lib/Transforms/NaCl/Convert...
lib/Transforms/NaCl/ConvertToPSO.cpp:67: LibraryDependencies("deps",
On 2016/03/29 15:29:39, Mark Seaborn wrote:
> Remember that this is an option for the 'opt' tool.  "-deps" is a bit too
> generic.  How about "-convert-to-pso-deps"?

Done.

https://codereview.chromium.org/1829793002/diff/1/lib/Transforms/NaCl/Convert...
lib/Transforms/NaCl/ConvertToPSO.cpp:663: for (unsigned i = 0; i !=
LibraryDependencies.size(); ++i) {
On 2016/03/29 15:29:39, Mark Seaborn wrote:
> You can use "for (... : LibrariesDependencies)"

Done.

https://codereview.chromium.org/1829793002/diff/1/lib/Transforms/NaCl/Convert...
lib/Transforms/NaCl/ConvertToPSO.cpp:699: // Dependencies List
On 2016/03/29 15:40:06, Mark Seaborn wrote:
> When you rebase, this should go at the end, after the TLS template fields, so
as
> not to break when rolled in.

Do you mean that I should place these fields after the
"PsoRoot.push_back(FieldVal)" below? This CL is up-to-date.

[Mark Seaborn, 2016-03-29 18:50:44]

https://codereview.chromium.org/1829793002/diff/1/lib/Transforms/NaCl/Convert...
File lib/Transforms/NaCl/ConvertToPSO.cpp (right):

https://codereview.chromium.org/1829793002/diff/1/lib/Transforms/NaCl/Convert...
lib/Transforms/NaCl/ConvertToPSO.cpp:699: // Dependencies List
On 2016/03/29 17:58:29, Sean Klein wrote:
> On 2016/03/29 15:40:06, Mark Seaborn wrote:
> > When you rebase, this should go at the end, after the TLS template fields,
so
> as
> > not to break when rolled in.
> 
> Do you mean that I should place these fields after the
> "PsoRoot.push_back(FieldVal)" below?

Yes, that would be better in the sense that it avoids the need for a 2-sided
change -- i.e. it avoids the need to update pll_root.h at the same time.  That
also means you can run the trybots for this change.

(We can reorder these fields to have a more coherent order at some point in the
future.)

> This CL is up-to-date.

Ah yes, I forgot that the TLS fields are added via push_back() rather than in
this initialiser.

[Sean Klein, 2016-03-29 19:50:03]

Description was changed from

==========
PNaCl Dynamic Linking: Storing dependencies from command line in PLL.

Used by https://codereview.chromium.org/1825893002/

TEST=run_pll_hello_world_test
BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4351
==========

to

==========
PNaCl Dynamic Linking: Storing dependencies from command line in PLL.

Used by https://codereview.chromium.org/1825893002/

TEST=run_pll_hello_world_test
BUG=https://bugs.chromium.org/p/nativeclient/issues/detail?id=4351
R=mseaborn@chromium.org

Committed:
https://chromium.googlesource.com/native_client/pnacl-llvm/+/3a2a7d006b87fc9f...
==========

[Sean Klein, 2016-03-29 19:50:05]

Committed patchset #2 (id:20001) manually as
3a2a7d006b87fc9ff029503b1f68d5f4428db4c7 (presubmit successful).