Prepare for LLVM 9 update

[nikic]

Main changes:

• In preparation for opaque pointer types, the byval attribute now takes a type. As such, the argument type needs to be threaded through to the function/callsite attribute application logic.
• On ARM the +fp-only-sp and +d16 features have become -fp64 and -d32. I've switched the target definitions to use the new names, but also added bidirectional emulation so either can be used on any LLVM version for backwards compatibility.
• The datalayout can now specify function pointer alignment. In particular on ARM Fi8 is specified, which means that function pointer alignment is independent of function alignment. I've added this to our datalayouts to match LLVM (which is something we check) and strip the fnptr alignment for older LLVM versions.
• The fmul/fadd reductions now always respect the accumulator (including for unordered reductions), so we should pass the identity instead of undef.

Open issues:

• https://reviews.llvm.org/D62106 causes linker errors with ld.bdf due to https://sourceware.org/bugzilla/show_bug.cgi?id=24784. To avoid this I've enabled RelaxELFRelocations, which results in a GOTPCRELX relocation for __tls_get_addr and avoids the issue. However, this is likely not acceptable because relax relocations are not supported by older linker versions. We may need an LLVM option to keep using PLT for __tls_get_addr despite RtLibUseGOT.

The corresponding llvm-project PR is rust-lang/llvm-project#19.

r? @ghost

[nikic]

@bors try

[bors]

⌛ Trying commit bf2edb0 with merge 3b203dc...

[bors]

💔 Test failed - checks-azure

[mati865]

/checkout/src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_posix.cc:349:59: error: use of undeclared identifier 'O_CLOEXEC'
      internal_open(shmname, O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, S_IRWXU));
                                                          ^
1 error generated.

O_CLOEXEC is available since Linux 2.6.23 but ancient CentoOS 5 used by the CI ships with Linux 2.6.18.

[nikic]

@bors try

I wasn't able to get the docker container running locally ... let's see if fixing the O_CLOEXEC is enough.

[bors]

⌛ Trying commit 39c7a58c39f215553ef0518ee88703eb86385287 with merge 82310f68ee567093e06ad767cc0d3cb6127457ba...

[alexcrichton]

This is looking great to me, thanks @nikic! I'm not entirely certain if the munging of features for ARM is worth it for older LLVM versions, but it seems pretty localized so not too bad to carry for a bit. I suspect these aren't the only feature names that have changed over time...

[bors]

☀️ Try build successful - checks-azure, checks-travis
Build commit: 82310f68ee567093e06ad767cc0d3cb6127457ba

[nikic]

I'm not entirely certain if the munging of features for ARM is worth it for older LLVM versions, but it seems pretty localized so not too bad to carry for a bit. I suspect these aren't the only feature names that have changed over time...

I've added this bit of emulation because these two features are used in some of the target definitions we ship, and I wasn't sure whether it's okay to make them incompatible with system LLVM.

@rust-timer build 82310f68ee567093e06ad767cc0d3cb6127457ba

[rust-timer]

Success: Queued 82310f68ee567093e06ad767cc0d3cb6127457ba with parent 78ca1bd, comparison URL.

[rust-timer]

Finished benchmarking try commit 82310f68ee567093e06ad767cc0d3cb6127457ba, comparison URL.

[mati865]

Note for people observating this: there are more optimisations in LLVM 9 so instruction count increase in opt jobs is expected.
To measure performance of the resulting binaries somebody would have to run lolbench.

[nikic]

The compile-time regressions for kekkac-opt and cranelift-codegen-opt at least look fairly bad though and should be investigated. Maybe there's some low hanging fruit.

[alexcrichton]

Our compatibility with the system LLVM is somewhat murkily defined. We haven't really ever supported it with 100% fidelity, but getting most tests to pass on "big targets" typically is necessary. For perhaps niche arm platforms and/or general platform supoprt 100% fidelity isn't required. In that sense it's not a bad thing to work with system LLVM's, but it's not always necessary since distributions (AFAIK), the primary use case, rarely cover the cross-compilation story affecting many of these targets.

[cuviper]

since distributions (AFAIK), the primary use case, rarely cover the cross-compilation story affecting many of these targets.

On Fedora/RHEL we don't cross-compile, but we do still care about some tier-2 platforms natively.

[nikic]

The compile-time regressions for kekkac-opt and cranelift-codegen-opt at least look fairly bad though and should be investigated. Maybe there's some low hanging fruit.

I checked keccak. Looks like the main regression there is in the SLP vectorizer, which went from 0.2s to something like 1.5s. BoUpSLP::getSpillCost() is the hottest LLVM function in the profile at around 6.5% overall.

[alexcrichton]

This seems like a great set of changes to land in the meantime even if we're hunting down LLVM issues, @nikic would you be up for landing everything here except the submodule update? We could then do that perhaps closer to the LLVM release or after we're confident in the current state of things.

[nikic]

This seems like a great set of changes to land in the meantime even if we're hunting down LLVM issues, @nikic would you be up for landing everything here except the submodule update? We could then do that perhaps closer to the LLVM release or after we're confident in the current state of things.

Sounds reasonable. I've now dropped the submodule update, as well as the SubtargetSubTypeKV change and the byval codegen test update. I think those two need to happen together with the LLVM update itself.

[nikic]

I've landed llvm/llvm-project@5ca39e8 to mitigate a large part of the SLP perf problem, though it doesn't fix the root cause.

[alexcrichton]

@bors: r+

FWIW that perf run looks excellent to me, so especially if you've mitigated the biggest regression (at least to some extent even if not the fullest) I'd be down for updating!

[bors]

📌 Commit ac56025 has been approved by alexcrichton

[RalfJung]

The datalayout can now specify function pointer alignment. In particular on ARM Fi8 is specified, which means that function pointer alignment is independent of function alignment. I've added this to our datalayouts to match LLVM (which is something we check) and strip the fnptr alignment for older LLVM versions.

Is that something Miri should add a check for when interpreting, to catch UB due to unaligned functions or so? I don't understand the distinction between "function alignment" and "function pointer alignment".

Currently, Miri treats function pointers as requiring alignment 1, i.e., there is no restriction.

[nikic]

The datalayout can now specify function pointer alignment. In particular on ARM Fi8 is specified, which means that function pointer alignment is independent of function alignment. I've added this to our datalayouts to match LLVM (which is something we check) and strip the fnptr alignment for older LLVM versions.

Is that something Miri should add a check for when interpreting, to catch UB due to unaligned functions or so? I don't understand the distinction between "function alignment" and "function pointer alignment".

Currently, Miri treats function pointers as requiring alignment 1, i.e., there is no restriction.

On most architectures function pointers have the same alignment as functions. On ARM, function pointers additionally store the ARM/Thumb mode in the low bit, so function pointers effectively become unaligned.

I don't know whether Miri can do anything interesting here...

[RalfJung]

On most architectures function pointers have the same alignment as functions.

And I take it that's not 1? Can I query that value from Miri somehow, i.e., is it available the way things like pointer_size are?

On ARM, function pointers additionally store the ARM/Thumb mode in the low bit, so function pointers effectively become unaligned.

I see. So e.g. the function might have to be 4-aligned but we can still see pointers that are just 1-aligned because the bits are used for Stuff (TM). Makes sense.

And yes checking that function pointers are aligned seems like something Miri can check for. Not sure yet how the alignment of functions themselves would come in though.

[nikic]

FWIW that perf run looks excellent to me, so especially if you've mitigated the biggest regression (at least to some extent even if not the fullest) I'd be down for updating!

I've opened a new PR for the update at #62592.

I've also submitted https://reviews.llvm.org/D64523 as a proper fix for the SLP vectorizer perf issue, but we can pull that in later. I looked at some of the other regressions as well, but wasn't really able to identify any specific source of slow-down.

[nikic]

On most architectures function pointers have the same alignment as functions.

And I take it that's not 1? Can I query that value from Miri somehow, i.e., is it available the way things like pointer_size are?

It's not right now, but should be simple to expose it, just one more case in the data layout parsing. Right now there aren't any targets that actually specify a non-trivial pointer alignment though.

[RalfJung]

Right now there aren't any targets that actually specify a non-trivial pointer alignment though.

Great, then right now there is nothing to do for Miri. :) Thanks!