Changes from 3.7 merge to files not in upstream

Changes from 3.7 merge to files not in upstream

[jvoung (off chromium), 2015-05-26 20:39:43]

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

[jvoung (off chromium), 2015-05-26 20:39:46]

Otherwise LGTM (yay!)

https://codereview.chromium.org/1151093004/diff/1/include/llvm/Bitcode/NaCl/N...
File include/llvm/Bitcode/NaCl/NaClBitcodeDist.h (right):

https://codereview.chromium.org/1151093004/diff/1/include/llvm/Bitcode/NaCl/N...
include/llvm/Bitcode/NaCl/NaClBitcodeDist.h:363: = delete;
nit: might be able to fit the = delete on the same line now

https://codereview.chromium.org/1151093004/diff/1/lib/Bitcode/NaCl/Reader/NaC...
File lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/lib/Bitcode/NaCl/Reader/NaC...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:1834: // MERGETODO(dschuff): does
this need to contain anything for TypeFinder?
can this TODO be removed now -- replace with comment about PNaCl restrictions?

https://codereview.chromium.org/1151093004/diff/1/lib/Target/JSBackend/JSBack...
File lib/Target/JSBackend/JSBackend.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/lib/Target/JSBackend/JSBack...
lib/Target/JSBackend/JSBackend.cpp:157: const DataLayout *DL;
Upstream whitespace change, or leave the space alone and keep lined up?

https://codereview.chromium.org/1151093004/diff/1/lib/Target/JSBackend/JSTarg...
File lib/Target/JSBackend/JSTargetTransformInfo.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/lib/Target/JSBackend/JSTarg...
lib/Target/JSBackend/JSTargetTransformInfo.cpp:1: //===--
JSTargetTransformInfo.cpp - JS specific TTI  ------------*- C++ -*-===//
don't need the *- C++ -* in a .cpp file (just .h)

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/Constan...
File lib/Transforms/NaCl/ConstantInsertExtractElementIndex.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/Constan...
lib/Transforms/NaCl/ConstantInsertExtractElementIndex.cpp:161: DL =
&BB.getParent()->getParent()->getDataLayout();
nit: Could be BB.getModule()->getDataLayout();

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/FixVect...
File lib/Transforms/NaCl/FixVectorLoadStoreAlignment.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/FixVect...
lib/Transforms/NaCl/FixVectorLoadStoreAlignment.cpp:251: DL =
&BB.getParent()->getParent()->getDataLayout();
could be getModule()

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/NoExitR...
File lib/Transforms/NaCl/NoExitRuntime.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/NoExitR...
lib/Transforms/NaCl/NoExitRuntime.cpp:1: //===- NoExitRuntime.cpp - Expand i64
and wider integer types -------------===//
comment seems to be copy-pasted, but I guess that's from emscripten?

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/Replace...
File lib/Transforms/NaCl/ReplacePtrsWithInts.cpp (left):

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/Replace...
lib/Transforms/NaCl/ReplacePtrsWithInts.cpp:594: // Now that all the replacement
instructions have been created, we
this is automatic now?

https://codereview.chromium.org/1151093004/diff/1/test/Transforms/NaCl/intern...
File test/Transforms/NaCl/internalize-used-globals.ll (right):

https://codereview.chromium.org/1151093004/diff/1/test/Transforms/NaCl/intern...
test/Transforms/NaCl/internalize-used-globals.ll:32: !llvm.ident = !{!0}
can this just be removed?

https://codereview.chromium.org/1151093004/diff/1/test/Transforms/NaCl/replac...
File test/Transforms/NaCl/replace-ptrs-with-ints.ll (left):

https://codereview.chromium.org/1151093004/diff/1/test/Transforms/NaCl/replac...
test/Transforms/NaCl/replace-ptrs-with-ints.ll:638: !4 = metadata !{metadata
!"0x2e\00nop\00nop\00\001\000\001\000\000\00256\001\001", metadata !1, metadata
!5, metadata !6, null, void (i8*)* @nop, null, null, metadata !9} ; [
DW_TAG_subprogram ] [line 1] [def] [nop]
If the test is now checking that "void (i32)* @debug_declare" shows up in the
metadata instead of nop (and instead of null, if that replaceFunction() call is
omitted), then you might be able to remove the @nop function test case, since it
isn't being checked in the subprograms anyway.

https://codereview.chromium.org/1151093004/diff/1/tools/pnacl-freeze/pnacl-fr...
File tools/pnacl-freeze/pnacl-freeze.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/tools/pnacl-freeze/pnacl-fr...
tools/pnacl-freeze/pnacl-freeze.cpp:82: if (!M.get()) {
could be the else { } from if (streamer), since it's checking the ErrorMessage
from getDataFileStreamer which is correlated with streamer. Otherwise, at this
point MOrErr is assumed to not be an Err because it's deref'ed as part of line
78?

[Derek Schuff, 2015-05-26 22:01:33]

https://codereview.chromium.org/1151093004/diff/1/include/llvm/Bitcode/NaCl/N...
File include/llvm/Bitcode/NaCl/NaClBitcodeDist.h (right):

https://codereview.chromium.org/1151093004/diff/1/include/llvm/Bitcode/NaCl/N...
include/llvm/Bitcode/NaCl/NaClBitcodeDist.h:363: = delete;
On 2015/05/26 20:39:45, jvoung wrote:
> nit: might be able to fit the = delete on the same line now

Done.

https://codereview.chromium.org/1151093004/diff/1/lib/Bitcode/NaCl/Reader/NaC...
File lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/lib/Bitcode/NaCl/Reader/NaC...
lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp:1834: // MERGETODO(dschuff): does
this need to contain anything for TypeFinder?
On 2015/05/26 20:39:45, jvoung wrote:
> can this TODO be removed now -- replace with comment about PNaCl restrictions?

Done.

https://codereview.chromium.org/1151093004/diff/1/lib/Target/JSBackend/JSBack...
File lib/Target/JSBackend/JSBackend.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/lib/Target/JSBackend/JSBack...
lib/Target/JSBackend/JSBackend.cpp:157: const DataLayout *DL;
On 2015/05/26 20:39:45, jvoung wrote:
> Upstream whitespace change, or leave the space alone and keep lined up?

Done.

https://codereview.chromium.org/1151093004/diff/1/lib/Target/JSBackend/JSTarg...
File lib/Target/JSBackend/JSTargetTransformInfo.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/lib/Target/JSBackend/JSTarg...
lib/Target/JSBackend/JSTargetTransformInfo.cpp:1: //===--
JSTargetTransformInfo.cpp - JS specific TTI  ------------*- C++ -*-===//
On 2015/05/26 20:39:45, jvoung wrote:
> don't need the *- C++ -* in a .cpp file (just .h)

Done.

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/Constan...
File lib/Transforms/NaCl/ConstantInsertExtractElementIndex.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/Constan...
lib/Transforms/NaCl/ConstantInsertExtractElementIndex.cpp:161: DL =
&BB.getParent()->getParent()->getDataLayout();
On 2015/05/26 20:39:45, jvoung wrote:
> nit: Could be BB.getModule()->getDataLayout();

Done.

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/FixVect...
File lib/Transforms/NaCl/FixVectorLoadStoreAlignment.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/FixVect...
lib/Transforms/NaCl/FixVectorLoadStoreAlignment.cpp:251: DL =
&BB.getParent()->getParent()->getDataLayout();
On 2015/05/26 20:39:45, jvoung wrote:
> could be getModule()

Done.

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/NoExitR...
File lib/Transforms/NaCl/NoExitRuntime.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/NoExitR...
lib/Transforms/NaCl/NoExitRuntime.cpp:1: //===- NoExitRuntime.cpp - Expand i64
and wider integer types -------------===//
On 2015/05/26 20:39:45, jvoung wrote:
> comment seems to be copy-pasted, but I guess that's from emscripten?

It is from emscripten, we should fix upstream

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/Replace...
File lib/Transforms/NaCl/ReplacePtrsWithInts.cpp (left):

https://codereview.chromium.org/1151093004/diff/1/lib/Transforms/NaCl/Replace...
lib/Transforms/NaCl/ReplacePtrsWithInts.cpp:594: // Now that all the replacement
instructions have been created, we
On 2015/05/26 20:39:45, jvoung wrote:
> this is automatic now?

RAUW seems to work on MetadataAsValue. We could maybe even get debug.value to
work now, but I'm not sure it's worth the effort.

https://codereview.chromium.org/1151093004/diff/1/test/Transforms/NaCl/intern...
File test/Transforms/NaCl/internalize-used-globals.ll (right):

https://codereview.chromium.org/1151093004/diff/1/test/Transforms/NaCl/intern...
test/Transforms/NaCl/internalize-used-globals.ll:32: !llvm.ident = !{!0}
On 2015/05/26 20:39:45, jvoung wrote:
> can this just be removed?

Done.

https://codereview.chromium.org/1151093004/diff/1/test/Transforms/NaCl/replac...
File test/Transforms/NaCl/replace-ptrs-with-ints.ll (left):

https://codereview.chromium.org/1151093004/diff/1/test/Transforms/NaCl/replac...
test/Transforms/NaCl/replace-ptrs-with-ints.ll:638: !4 = metadata !{metadata
!"0x2e\00nop\00nop\00\001\000\001\000\000\00256\001\001", metadata !1, metadata
!5, metadata !6, null, void (i8*)* @nop, null, null, metadata !9} ; [
DW_TAG_subprogram ] [line 1] [def] [nop]
On 2015/05/26 20:39:45, jvoung wrote:
> If the test is now checking that "void (i32)* @debug_declare" shows up in the
> metadata instead of nop (and instead of null, if that replaceFunction() call
is
> omitted), then you might be able to remove the @nop function test case, since
it
> isn't being checked in the subprograms anyway.

Done.

https://codereview.chromium.org/1151093004/diff/1/tools/pnacl-freeze/pnacl-fr...
File tools/pnacl-freeze/pnacl-freeze.cpp (right):

https://codereview.chromium.org/1151093004/diff/1/tools/pnacl-freeze/pnacl-fr...
tools/pnacl-freeze/pnacl-freeze.cpp:82: if (!M.get()) {
On 2015/05/26 20:39:45, jvoung wrote:
> could be the else { } from if (streamer), since it's checking the ErrorMessage
> from getDataFileStreamer which is correlated with streamer. Otherwise, at this
> point MOrErr is assumed to not be an Err because it's deref'ed as part of line
> 78?

Done.