Skip to content
This repository was archived by the owner on Feb 5, 2019. It is now read-only.

PNaCl Transform Passes for Rust. #6

Conversation

DiamondLovesYou
Copy link

This (rather large) PR is mostly a set of cherry picked commits from Google PNaCl 's LLVM fork, with the rest consisting of additional fixes plus supplemental transform passes I wrote for my PNaCl Rust branch.
Patches to Rust proper for PNaCl targets are forthcoming.

Mark Seaborn and others added 30 commits February 25, 2014 13:26
…d results

This pass expands out the "byval" and "sret" argument attributes.

This will affect the calling conventions for PPAPI under PNaCl (for
passing PP_Var etc. by value), so the PNaCl PPAPI shims will need to
be updated in order to enable this pass by default.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3400
TEST=PNaCl toolchain trybots + GCC torture tests + LLVM test suite + Spec2k

Review URL: https://codereview.chromium.org/13973018

(cherry picked from commit ac21bcb)
Copy classes for LLVM BitcodeWriter into a PNaCL subdirectory, to create a
PNaCL version. Renames classes/methods to include PNaCl prefix so that they
don't conflict with the LLVM BitcodeWriter.

Also removed experimental support for use-list order
preservation.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3405

Review URL: https://codereview.chromium.org/14126011

(cherry picked from commit 5712db9)

Conflicts:
	tools/LLVMBuild.txt
	tools/Makefile
Disallow all metadata by default.  There is a flag "-allow-debug-metadata",
which will be used in pnacl-ld driver, to not change the debugging workflow.
That flag will not be present in the pnacl-abicheck driver though.

We'll also run -strip-metadata within pnacl-ld, after optimizations are run,
so that at least that part is checked inside pnacl-ld.
CL for driver changes:
https://codereview.chromium.org/14358048/

BUG= http://code.google.com/p/nativeclient/issues/detail?id=3348
[email protected]

Review URL: https://codereview.chromium.org/14329025

(cherry picked from commit f8b761e)
… types.

Saves a tiny bit of space for var-args heavy bitcode
programs, since the anonymous types get unique'ed.  E.g.,
saves 20KB out of 1.6MB in spec gcc when comparing the
gzipped files, or about 100KB when not zipped.  This is only
a savings with the current wire format.  If we change the
alloca, etc. to only have sizes and not struct types
then we would also not have this duplication.

Just happened to notice while looking through code for
what struct types remain used in the bitcode.

random cleanup for:
BUG= https://code.google.com/p/nativeclient/issues/detail?id=3338
[email protected]

Review URL: https://codereview.chromium.org/14197004

(cherry picked from commit 1174111)
Copy classes for LLVM BitcodeReader into a NaCl subdirectory, to create a wire
format version. Renames classes/functions to include NaCl prefix, so that
they don't conflict with the LLVM Bitcode reader.

Also implements pnacl-thaw, showing that we can read the PNaCl wire format
files.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3405
[email protected]

Review URL: https://codereview.chromium.org/14314016

(cherry picked from commit f42b26d)

Conflicts:
	tools/CMakeLists.txt
	tools/Makefile
Create NaCl specific version of BitCodes.h so that we can custimize it
for the PNaCl wire format. Moves enums to namespace naclbitc. Renames
classes using NaCl prefix. Fixes references so that files compiles.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3405
[email protected]

Review URL: https://codereview.chromium.org/14682013

(cherry picked from commit 7ecb66d)
This list is currently too small to support our tests
(scons, gcc, llvm). To prevent the tests from breaking,
there is a -pnaclabi-allow-intrinsics flag which defaults
to true.  To turn on the checking for real,
set -pnaclabi-allow-intrinsics=0.

We will avoid actually using the -pnaclabi-allow-intrinsics
flags in tests, and try to just stick with the
-pnacl-disable-abi-check flag. Remove this flag soon.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3378
[email protected]

Review URL: https://codereview.chromium.org/14670017

(cherry picked from commit 77cc10f)
This IR pass for ARM inserts a comparison and a branch to trap if the
denominator of a DIV or REM instruction is zero.  This makes ARM fault
identically to x86 in this case.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=2833
[email protected]

Review URL: https://codereview.chromium.org/14607004

(cherry picked from commit 8c9803a)
This is used by the LLVM translator as part of:
lib/Analysis/ConstantFolding.cpp (it tries to do constant
folding for pow calls...)

Also, tweak comment about llvm.pow vs llvm.powi.  It
looks like powi is the imprecise one, not pow.

BUG=unblock DEPs roll, broken self-build.
BUG=http://code.google.com/p/nativeclient/issues/detail?id=3378
[email protected]

Review URL: https://codereview.chromium.org/14631013

(cherry picked from commit e6d4584)
This could help prevent these expansion passes from inhibiting
optimisations than run after the expansion.  e.g. It gives the
optimiser more freedom to move around reads from the varargs buffer
because they will not alias writes to other locations.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3400
TEST=PNaCl toolchain trybots + GCC torture tests + LLVM test suite + Spec2k

Review URL: https://codereview.chromium.org/14060026

(cherry picked from commit 0e6d484)
Tests that use them now skip ABI verification.
Known tests that use this are the scons tests:
- the barebones_stack_alignment16 test and
- the EH ones under tests/toolchain/

Also list other eh_* in the blacklist more explicitly
(they were already caught by the default case).

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3378
TEST= scons, gcc torture, llvm
[email protected]

Review URL: https://codereview.chromium.org/14998008

(cherry picked from commit 4d0b8cd)
This pass (mostly) legalizes integer types by promoting them.
It has some limitations (e.g. it can't change function types)
but it is more than sufficient for what clang and SROA generate.

A more significant limitation of promotion is that packed
bitfields of size > 64 bits are still not handled. There are
none in our tests (other than callingconv_case_by_case which
doesn't require a stable pexe) but we will want to either
handle them by correctly expanding them, or find a better way
to error out.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3360
[email protected], [email protected]

Review URL: https://codereview.chromium.org/14569012

(cherry picked from commit a0efa09)
…cker.

They do not seem to be widely used by user code.

* The boehm garbage collector library does reference
__builtin_return_address under an ifdef, but it
does not appear to be compiled in.

* Mesa-7.6 uses __builtin_frame_address
for u_debug_stack.c, but that also does not appear to be
part of the built libraries.

They expose stack/code addresses (at least the lower
32-bits of the address).

As part of https://codereview.chromium.org/14619022/,
we stopped considering the scons and gcc torture tests that
use these intrinsics as meeting the stable ABI.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3378
[email protected]

Review URL: https://codereview.chromium.org/14657017

(cherry picked from commit 33da0d9)
    Intrinsic::getDeclaration and use the definition in
    include/llvm/Intrinsics.td

    This also makes the attribute on the intrinsic to be more
    consistent with the back-end (code-gen), which automatically assumes
    it's ReadNone (because this is what Intrinsics.td) defines.

    Using ReadNone rather than ReadOnly may be not strictly correct because
    the intrinsic depends on the value of the TP. However, this attribute is
    not really used anywhere in IR optimizations, and in the backend the
    intrinsic is ReadNone anyhow (the IR setting gets overridden).

    If we run into any problems with this in the future, we may consider
    handling the lowering of this intrinsic in
    TargetLowering::LowerINTRINSIC_W_CHAIN rather than in
    TargetLowering::LowerINTRINSIC_WO_CHAIN.

BUG=None
[email protected]

Review URL: https://codereview.chromium.org/14643019

(cherry picked from commit 8fe6f1f)

Conflicts:
	include/llvm/IR/Intrinsics.td
…ables

If a global variable has no "align" attribute, it must be aligned
based on its type.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3437
TEST=flatten-globals.ll

Review URL: https://codereview.chromium.org/15359006

(cherry picked from commit 6aea8b4)
Create type IDs based on number of references, rather than first reached.
This is done so that fewer bits are used to encode types that are commonly
used.

Note that this cuts the size of the generate bitcode file by about 1.5%, with
no effect on the reader, since it only changes the order type ID's are created.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3405
[email protected]

Review URL: https://codereview.chromium.org/14495008

(cherry picked from commit 1046c28)
While writing a test, I noticed that ExpandCtors crashes if given the
empty list "[]", because this gets converted into an UndefValue
ConstantExpr by the LLVM assembly reader.

Fix this by checking the array's size via its type.  This replaces the
isNullValue() check.

Make error handling cleaner by splitting out a separate function.

BUG=none
TEST=test/Transforms/NaCl/expand-ctors-emptylist.ll

Review URL: https://codereview.chromium.org/15659005

(cherry picked from commit 099f6bf)
ReplacePtrsWithInts converts IR to a normal form in which functions
don't reference any aggregate pointer types and pointer types only
appear inside a few instructions.

Change BlockAddress::replaceUsesOfWithOnConstant() to handle changing
a function's type by replacing a function with a bitcast ConstantExpr
of a new function.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3343
TEST=replace-ptrs-with-ints.ll + PNaCl toolchain trybots, torture tests, etc.

Review URL: https://codereview.chromium.org/14262011

(cherry picked from commit 9f0ec13)

Conflicts:
	lib/IR/Constants.cpp
Disallow i1, i8, which don't make sense for "byte" swapping.
None of these generate outcalls to compiler_rt.

Test coverage:
* test/CodeGen/AArch64/dp1.ll (only testing i32, i64)
* test/CodeGen/ARM/rev.ll (only testing i16)
* test/CodeGen/Mips/bswap.ll (only testing i32, i64)
* test/CodeGen/X86/bswap.ll (i16, i32, i64, on i686)
* test/NaCl/{ARM,X86}/intrinsics-bitmanip.ll (i16, i32, i64)
(maybe the last set of tests could be merged into the
upstream tests)

For targets without native i16 bswap, the i16 could just be
a bswap on an i32 plus a shift right 16.

Other test coverage:
* gcc/testsuite/gcc.dg/builtin-bswap-[1,2,3,4,5].c

Misc: style cleanups, and add comments to the undocumented intrinsics in the dev list.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3378
[email protected]

Review URL: https://codereview.chromium.org/14914011

(cherry picked from commit 5019000)
[1] Explicitly enumerate all abbreviation values, including the
maximum abbreviation for each type of block.

[2] Make "enter subblock" calculate number of bits needed by passing
in maximum abbreviation (associated with block) rather than requiring
the developer to compute this value every time a subblock is entered.

*NOTE* This code changes encoding sizes to be based on
the maximum allowed value, rather than requiring the
developer to calculate out the number of bits needed. This
change doesn't make  the PNaCL bitcode files
incompatable with LLVM bitcode files, since it does
not effect the bitcode reader.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3405
[email protected]

Review URL: https://codereview.chromium.org/14813032

(cherry picked from commit 80b7ba7)
@alexcrichton
Copy link
Member

I would be pretty uncomfortable diverging from LLVM by this much. We're getting to the point where we're almost compatible with vanilla upstream LLVM, so it seems a bit of a shame to include all these patches on top.

I would almost rather improve the --llvm-root configure switch so you're able to have your own build of LLVM locally and use that instead.

@DiamondLovesYou
Copy link
Author

Ah, I didn't realize that. Well, I did deliberately confine the cherry picking (I wanted to preserve the history as much as possible) to commits regarding passes or bitcode reading/writing, so there shouldn't be any breaking C API changes as compared to before, other than the additional initialize_Pass, PNaClABISimplifyAddPreOptPasses, && PNaClABISimplifyAddPostOptPasses functions.
Regarding PNaClABISimplifyAddPreOptPasses && PNaClABISimplifyAddPostOptPasses, they're just convenience functions, so they can be replaced by adding the respective passes to the pass manager by name when needed (ie only while targeting le32-unknown-nacl). As for the initialize_Pass functions, we could maintain vanilla compatibility by declaring the proper prototypes in RustWrapper.cpp with weak linkage and then check them against null before calling.
However, after doing some research, this doesn't seem like such the savior as I had hoped: I haven't found any docs indicating support for it in Clang, additionally it requires use of sos or dylibs or requires linker flags when used with static archives (the latter moves potential linker errors to runtime); so generally no bueno.
AIUI, the only option left is a HAVE_CONFIG_H style define, with stubs in RustWrapper.cpp that always signal error when called in the case of a missing config define. This is ugly.
Perhaps, given LLVM's popularity and the fact that everybody and their dog has a fork, it might be worthwhile to create and upstream a C API system that allows forks to have their own initialization routines for passes or what-have-you without affecting overall API compatibility. Though, somewhat ironically, it would still suffer from API incompatibility until downstreamed in-volume, so it wouldn't be immediately employable.

Regarding your second thought, I'm already working on a project which would nicely address it, though it's still in its infancy and quite off-topic.

@alexcrichton
Copy link
Member

It sounds like this is a pretty specific use case for LLVM, and I'm not sure that we want to start maintaining so many patches without an official use case. It's already difficult for us to maintain the few patches we have as upstream LLVM changes, so I don't think we want to pile on this amount of code that we don't fully understand.

I would highly recommend using --llvm-root for a use case such as this, at least as a starting point.

alexcrichton pushed a commit that referenced this pull request Apr 14, 2014
This requires a number of steps.
1) Move value_use_iterator into the Value class as an implementation
   detail
2) Change it to actually be a *Use* iterator rather than a *User*
   iterator.
3) Add an adaptor which is a User iterator that always looks through the
   Use to the User.
4) Wrap these in Value::use_iterator and Value::user_iterator typedefs.
5) Add the range adaptors as Value::uses() and Value::users().
6) Update *all* of the callers to correctly distinguish between whether
   they wanted a use_iterator (and to explicitly dig out the User when
   needed), or a user_iterator which makes the Use itself totally
   opaque.

Because #6 requires churning essentially everything that walked the
Use-Def chains, I went ahead and added all of the range adaptors and
switched them to range-based loops where appropriate. Also because the
renaming requires at least churning every line of code, it didn't make
any sense to split these up into multiple commits -- all of which would
touch all of the same lies of code.

The result is still not quite optimal. The Value::use_iterator is a nice
regular iterator, but Value::user_iterator is an iterator over User*s
rather than over the User objects themselves. As a consequence, it fits
a bit awkwardly into the range-based world and it has the weird
extra-dereferencing 'operator->' that so many of our iterators have.
I think this could be fixed by providing something which transforms
a range of T&s into a range of T*s, but that *can* be separated into
another patch, and it isn't yet 100% clear whether this is the right
move.

However, this change gets us most of the benefit and cleans up
a substantial amount of code around Use and User. =]

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@203364 91177308-0d34-0410-b5e6-96231b3b80d8
alexcrichton pushed a commit that referenced this pull request Aug 2, 2018
…d VPlan for tests."

Memory leaks in tests.
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/6289/steps/check-llvm%20asan/logs/stdio

Direct leak of 192 byte(s) in 1 object(s) allocated from:
    #0 0x554ea8 in operator new(unsigned long) /b/sanitizer-x86_64-linux-bootstrap/build/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:106
    #1 0x56cef1 in llvm::VPlanTestBase::doAnalysis(llvm::Function&) /b/sanitizer-x86_64-linux-bootstrap/build/llvm/unittests/Transforms/Vectorize/VPlanTestBase.h:53:14
    #2 0x56bec4 in llvm::VPlanTestBase::buildHCFG(llvm::BasicBlock*) /b/sanitizer-x86_64-linux-bootstrap/build/llvm/unittests/Transforms/Vectorize/VPlanTestBase.h:57:3
    #3 0x571f1e in llvm::(anonymous namespace)::VPlanHCFGTest_testVPInstructionToVPRecipesInner_Test::TestBody() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp:119:15
    #4 0xed2291 in testing::Test::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc
    #5 0xed44c8 in testing::TestInfo::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11
    #6 0xed5890 in testing::TestCase::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28
    #7 0xef3634 in testing::internal::UnitTestImpl::RunAllTests() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43
    #8 0xef27e0 in testing::UnitTest::Run() /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/src/gtest.cc
    #9 0xebbc23 in RUN_ALL_TESTS /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46
    #10 0xebbc23 in main /b/sanitizer-x86_64-linux-bootstrap/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:51
    #11 0x7f65569592e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)

and more.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@336718 91177308-0d34-0410-b5e6-96231b3b80d8
alexcrichton pushed a commit that referenced this pull request Aug 2, 2018
…ering"

This reverts commit r337021.

WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x1415cd65 in void write_signed<long>(llvm::raw_ostream&, long, unsigned long, llvm::IntegerStyle) /code/llvm-project/llvm/lib/Support/NativeFormatting.cpp:95:7
    #1 0x1415c900 in llvm::write_integer(llvm::raw_ostream&, long, unsigned long, llvm::IntegerStyle) /code/llvm-project/llvm/lib/Support/NativeFormatting.cpp:121:3
    #2 0x1472357f in llvm::raw_ostream::operator<<(long) /code/llvm-project/llvm/lib/Support/raw_ostream.cpp:117:3
    #3 0x13bb9d4 in llvm::raw_ostream::operator<<(int) /code/llvm-project/llvm/include/llvm/Support/raw_ostream.h:210:18
    #4 0x3c2bc18 in void printField<unsigned int, &(amd_kernel_code_s::amd_kernel_code_version_major)>(llvm::StringRef, amd_kernel_code_s const&, llvm::raw_ostream&) /code/llvm-project/llvm/lib/Target/AMDGPU/Utils/AMDKernelCodeTUtils.cpp:78:23
    #5 0x3c250ba in llvm::printAmdKernelCodeField(amd_kernel_code_s const&, int, llvm::raw_ostream&) /code/llvm-project/llvm/lib/Target/AMDGPU/Utils/AMDKernelCodeTUtils.cpp:104:5
    #6 0x3c27ca3 in llvm::dumpAmdKernelCode(amd_kernel_code_s const*, llvm::raw_ostream&, char const*) /code/llvm-project/llvm/lib/Target/AMDGPU/Utils/AMDKernelCodeTUtils.cpp:113:5
    #7 0x3a46e6c in llvm::AMDGPUTargetAsmStreamer::EmitAMDKernelCodeT(amd_kernel_code_s const&) /code/llvm-project/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp:161:3
    #8 0xd371e4 in llvm::AMDGPUAsmPrinter::EmitFunctionBodyStart() /code/llvm-project/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:204:26

[...]

Uninitialized value was created by an allocation of 'KernelCode' in the stack frame of function '_ZN4llvm16AMDGPUAsmPrinter21EmitFunctionBodyStartEv'
    #0 0xd36650 in llvm::AMDGPUAsmPrinter::EmitFunctionBodyStart() /code/llvm-project/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:192

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@337079 91177308-0d34-0410-b5e6-96231b3b80d8
Sirokujira pushed a commit to Sirokujira/llvm that referenced this pull request Dec 3, 2018
…>> (32 - y) pattern"

*Seems* to be breaking sanitizer-x86_64-linux-fast buildbot,
the ELF/relocatable-versioned.s test:

==17758==MemorySanitizer CHECK failed: /b/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_allocator.cc:191 "((kBlockMagic)) == ((((u64*)addr)[0]))" (0x6a6cb03abcebc041, 0x0)
    #0 0x59716b in MsanCheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /b/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/msan/msan.cc:393
    rust-lang#1 0x586635 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /b/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_termination.cc:79
    rust-lang#2 0x57d5ff in __sanitizer::InternalFree(void*, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<__sanitizer::AP32> >*) /b/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_allocator.cc:191
    rust-lang#3 0x7fc21b24193f  (/lib/x86_64-linux-gnu/libc.so.6+0x3593f)
    rust-lang#4 0x7fc21b241999 in exit (/lib/x86_64-linux-gnu/libc.so.6+0x35999)
    rust-lang#5 0x7fc21b22c2e7 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e7)
    rust-lang#6 0x57c039 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/lld+0x57c039)

This reverts commit r345014.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@345017 91177308-0d34-0410-b5e6-96231b3b80d8
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants