Default relax_elf_relocations to true

[MaskRay]

This option tells LLVM to emit relaxable relocation types R_X86_64_GOTPCRELX/R_X86_64_REX_GOTPCRELX/R_386_GOT32X in applicable cases. True matches Clang's CMake default since 2020-08 1 and latest LLVM default2.

This also works around a GNU ld<2.41 issue2 when using general-dynamic/local-dynamic TLS models in -Z plt=no mode with latest LLVM.

[rustbot]

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

⚠️ Warning ⚠️

• These commits modify submodules.

[rustbot]

These commits modify compiler targets.
(See the Target Tier Policy.)

[MaskRay]

failures:
    [mir-opt] src/test/mir-opt/generator_tiny.rs
    [mir-opt] src/test/mir-opt/simplify_cfg.rs
    [mir-opt] src/test/mir-opt/sroa.rs
...
TLS transition from R_386_TLS_GD to R_386_TLS_IE_32 against ... in section `.text' failed

are due to the current -Z plt=no default doesn't play well with GNU ld. I think this will be fixed when -Z plt=no default is reverted. See #106380

I try to https://sourceware.org/pipermail/binutils/2023-January/125507.html despite closed https://sourceware.org/bugzilla/show_bug.cgi?id=24784 .

[nikic]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 9a9ba97d5d67de51d8acc68d546d97f74bc04b92 with merge e0dceaae06dc2a3d7083c69129e9d7826eb21aae...

[bors]

☀️ Try build successful - checks-actions
Build commit: e0dceaae06dc2a3d7083c69129e9d7826eb21aae (e0dceaae06dc2a3d7083c69129e9d7826eb21aae)

[rust-timer]

Finished benchmarking commit (e0dceaae06dc2a3d7083c69129e9d7826eb21aae): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.8%, 0.8%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.8%	[0.8%, 0.8%]	1

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.1%	[-0.1%, -0.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.1%	[-0.1%, -0.1%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

[nikic]

Minor code size improvements, no notable effects otherwise.

It sounds like we can't enable this for i386 right now, so possibly enable it only for x86_64 targets?

What kind of linker requirement does that impose (i.e. which is the earliest version of ld.bfd, ld.golf, ld.lld that reliably supports this and how old are they?)

[MaskRay]

Minor code size improvements, no notable effects otherwise.

It sounds like we can't enable this for i386 right now, so possibly enable it only for x86_64 targets?

What kind of linker requirement does that impose (i.e. which is the earliest version of ld.bfd, ld.golf, ld.lld that reliably supports this and how old are they?)

I want to avoid the configuration complexity mess. This should just be the default for both x86-32 and x86-64.
For GNU ld and gold, binutils 2.26 has the support (the commits are around 2015-10).
lld decent support for R_386_GOT32X is perhaps 2017-04 (llvm-project e7bf968). Note that rustc's default debug info cannot use lld<12 anyway (2020-11 https://reviews.llvm.org/D91291).

I believe the i386 issue will be resolved if we restore the -Z plt=yes sensible default (matching GCC and Clang).

[davidtwco]

r? @nikic

[nikic]

failures:
    [mir-opt] src/test/mir-opt/generator_tiny.rs
    [mir-opt] src/test/mir-opt/simplify_cfg.rs
    [mir-opt] src/test/mir-opt/sroa.rs
...
TLS transition from R_386_TLS_GD to R_386_TLS_IE_32 against ... in section `.text' failed

are due to the current -Z plt=no default doesn't play well with GNU ld. I think this will be fixed when -Z plt=no default is reverted. See #106380

Do you know whether there is a binutils bug for this? I found https://sourceware.org/bugzilla/show_bug.cgi?id=9938, but that's really old. From https://sourceware.org/pipermail/binutils/2023-January/125506.html it seems like maintainers are arguing that -fno-plt is only supported in conjunction with -mrelax-relocations=yes on x86_64 -- but it looks like at the same time -fno-plt only works with -mrelax-relocations=no on i686? Or am I misunderstanding something here?

[MaskRay]

failures:
    [mir-opt] src/test/mir-opt/generator_tiny.rs
    [mir-opt] src/test/mir-opt/simplify_cfg.rs
    [mir-opt] src/test/mir-opt/sroa.rs
...
TLS transition from R_386_TLS_GD to R_386_TLS_IE_32 against ... in section `.text' failed

are due to the current -Z plt=no default doesn't play well with GNU ld. I think this will be fixed when -Z plt=no default is reverted. See #106380

Do you know whether there is a binutils bug for this? I found https://sourceware.org/bugzilla/show_bug.cgi?id=9938, but that's really old. From https://sourceware.org/pipermail/binutils/2023-January/125506.html it seems like maintainers are arguing that -fno-plt is only supported in conjunction with -mrelax-relocations=yes on x86_64 -- but it looks like at the same time -fno-plt only works with -mrelax-relocations=no on i686? Or am I misunderstanding something here?

I don't know. https://sourceware.org/bugzilla/show_bug.cgi?id=24784 applies to x86-32 as well.

My stance is pretty clear: (a) default to -Z plt=yes, (b) enable relax_elf_relocations, (c) revert your revert of my llvm-project commit.
If Rust applies the patches in this order, there should be no issue.

[nikic]

failures:
    [mir-opt] src/test/mir-opt/generator_tiny.rs
    [mir-opt] src/test/mir-opt/simplify_cfg.rs
    [mir-opt] src/test/mir-opt/sroa.rs
...
TLS transition from R_386_TLS_GD to R_386_TLS_IE_32 against ... in section `.text' failed

are due to the current -Z plt=no default doesn't play well with GNU ld. I think this will be fixed when -Z plt=no default is reverted. See #106380

Do you know whether there is a binutils bug for this? I found https://sourceware.org/bugzilla/show_bug.cgi?id=9938, but that's really old. From https://sourceware.org/pipermail/binutils/2023-January/125506.html it seems like maintainers are arguing that -fno-plt is only supported in conjunction with -mrelax-relocations=yes on x86_64 -- but it looks like at the same time -fno-plt only works with -mrelax-relocations=no on i686? Or am I misunderstanding something here?

I don't know. https://sourceware.org/bugzilla/show_bug.cgi?id=24784 applies to x86-32 as well.

But that issue is about the -mrelax-relocations=no case, while we're testing -mrelax-relocations=yes here, so those shouldn't be related?

[apiraino]

Switching to waiting on author as I think it was reviewed. Feel free to request a review with @rustbot ready, thanks!

@rustbot author

[MaskRay]

Rebased after upgrading binutils for FreeBSD and Illumos

[nikic]

@bors r+ rollup=never

[bors]

📌 Commit f3d8191 has been approved by nikic

It is now in the queue for this repository.

[bors]

⌛ Testing commit f3d8191 with merge 4410868...

[bors]

☀️ Test successful - checks-actions
Approved by: nikic
Pushing 4410868 to master...

[rust-timer]

Finished benchmarking commit (4410868): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.5%, 0.9%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.7%	[0.5%, 0.9%]	2

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.6%	[-5.5%, -0.1%]	3
Improvements ✅ (secondary)	-3.4%	[-3.9%, -2.9%]	2
All ❌✅ (primary)	-2.6%	[-5.5%, -0.1%]	3

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.6%	[-0.9%, -0.4%]	16
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.6%	[-0.9%, -0.4%]	16

Bootstrap: 635.214s -> 635.876s (0.10%)
Artifact size: 347.18 MiB -> 347.03 MiB (-0.04%)

[Enselic]

This commit has been confirmed to introduce a regression with the mold linker. See https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/SIGILL.20in.20build-script-build.20with.20nightly-2023-08-25/near/387506479.