change offset from u32 to u64

[Dylan-DPC-zz]

References #71696

r? @oli-obk

(closed the earlier pr because the rebase got messed up)

[pickfire]

Why shouldn't it be limited to usize?

[Mark-Simulacrum]

@bors try @rust-timer queue

The bump in PlaceElem size here for the very unlikely case of wanting more than 2^32 elements in a slice or so seems pretty unfortunate.

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit f6a4a76 with merge 8b3c4d181991e5ec1946c4e1a4125b118dc26bb0...

[elichai]

Why shouldn't it be limited to usize?

Because it should support whatever offset the target supports.
So if you run rustc on a 32bit but target 64bit offsets can be 64bit, not 32bit.
I think this should be as big as the biggest architecture pointer supported.

[pickfire]

Because it should support whatever offset the target supports.
So if you run rustc on a 32bit but target 64bit offsets can be 64bit, not 32bit.
I think this should be as big as the biggest architecture pointer supported.

Then why isn't it 128 bit?

[bors]

☀️ Try build successful - checks-actions, checks-azure
Build commit: 8b3c4d181991e5ec1946c4e1a4125b118dc26bb0 (8b3c4d181991e5ec1946c4e1a4125b118dc26bb0)

[rust-timer]

Queued 8b3c4d181991e5ec1946c4e1a4125b118dc26bb0 with parent 5890563, future comparison URL.

[tesuji]

Then why isn't it 128 bit

Rust hasn't ever supported 128 pointer width.

[rust-timer]

Finished benchmarking try commit (8b3c4d181991e5ec1946c4e1a4125b118dc26bb0): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never

[Mark-Simulacrum]

Performance looks like a slight (maybe .1%) loss on most benchmarks. The original downsizing was in #55589, by @oli-obk -- I suspect we didn't individually benchmark the various enums changing sizes, so maybe PlaceElem is not that important (or has become less important since then).

[oli-obk]

Yea, from the perf results I'd say let's do this (since it reduces complexity and possible ICEs)

@bors r+

[bors]

📌 Commit f6a4a76 has been approved by oli-obk

[bors]

⌛ Testing commit f6a4a76 with merge 6ead622...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: oli-obk
Pushing 6ead622 to master...

[rust-log-analyzer]

Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @rust-lang/infra. (Feature Requests)