https://github.com/rust-lang/rust http://git.dreamy.place/mirrors/rust/atom?h=1.78.0 2024-03-11T04:45:27+00:00 2024-03-11T04:45:27+00:00 bors bors@rust-lang.org 2024-03-11T04:45:27+00:00 urn:sha1:a6d93acf5fdeb020ab86cc0d30d5672c23a7dba6 Stop using LLVM struct types for byval/sret For `byval` and `sret`, the type has no semantic meaning, only the size matters\*†. Using `[N x i8]` is a more direct way to specify that we want `N` bytes, and avoids relying on LLVM's struct layout. \*: The alignment would matter, if we didn't explicitly specify it. From what I can tell, we always specified the alignment for `sret`; for `byval`, we didn't until #112157. †: For `byval`, the hidden copy may be impacted by padding in the LLVM struct type, i.e. padding bytes may not be copied. (I'm not sure if this is done today, but I think it would be legal.) But we manually pad our LLVM struct types specifically to avoid there ever being LLVM-visible padding, so that shouldn't be an issue. Split out from #121577. r? `@nikic` 2024-03-05T23:54:45+00:00 Erik Desjardins erikdesjardins@users.noreply.github.com 2024-02-25T05:43:03+00:00 urn:sha1:96a72676d1134f58c378b0b8003922b09b9f1e5f This avoids depending on LLVM's struct types to determine the size of the byval/sret slot. 2024-03-05T14:03:33+00:00 Ralf Jung post@ralfj.de 2024-03-05T10:32:03+00:00 urn:sha1:f391c0793b443d30ef8c4d4228550439d4dbfead 2024-02-25T11:03:48+00:00 Markus Reiter me@reitermark.us 2024-02-22T13:59:52+00:00 urn:sha1:b2fbb8a05392be976c67e3b0063203d5b049da5c 2024-02-22T16:04:04+00:00 许杰友 Jieyou Xu (Joe) jieyouxu@outlook.com 2024-02-22T12:10:29+00:00 urn:sha1:6e48b96692d63a79a14563f27fe5185f122434f8 2023-12-15T16:42:05+00:00 Nikita Popov npopov@redhat.com 2023-12-15T11:14:39+00:00 urn:sha1:c2fd26a115645c92537719b1a04270e1ba727cbf Currently, we assume that ScalarPair is always represented using a two-element struct, both as an immediate value and when stored in memory. This currently works fairly well, but runs into problems with https://github.com/rust-lang/rust/pull/116672, where a ScalarPair involving an i128 type can no longer be represented as a two-element struct in memory. For example, the tuple `(i32, i128)` needs to be represented in-memory as `{ i32, [3 x i32], i128 }` to satisfy alignment requirement. Using `{ i32, i128 }` instead will result in the second element being stored at the wrong offset (prior to LLVM 18). Resolve this issue by no longer requiring that the immediate and in-memory type for ScalarPair are the same. The in-memory type will now look the same as for normal struct types (and will include padding filler and similar), while the immediate type stays a simple two-element struct type. This also means that booleans in immediate ScalarPair are now represented as i1 rather than i8, just like we do everywhere else. The core change here is to llvm_type (which now treats ScalarPair as a normal struct) and immediate_llvm_type (which returns the two-element struct that llvm_type used to produce). The rest is fixing things up to no longer assume these are the same. In particular, this switches places that try to get pointers to the ScalarPair elements to use byte-geps instead of struct-geps. 2023-07-27T21:44:13+00:00 Josh Stone jistone@redhat.com 2023-07-27T21:44:13+00:00 urn:sha1:da47736f42307e450a447889ebc563cddaf93ac2 2023-07-10T23:19:30+00:00 Patrick Walton pcwalton@fb.com 2022-11-01T03:38:40+00:00 urn:sha1:0becc89d4a75d14571b02fb34ec1e3a45c9fb9dc alignment of `byval` on x86 in the process. Commit 88e4d2c2918428d55e34cd57c11279ea839c8822 from five years ago removed support for alignment on indirectly-passed arguments because of problems with the `i686-pc-windows-msvc` target. Unfortunately, the `memcpy` optimizations I recently added to LLVM 16 depend on this to forward `memcpy`s. This commit attempts to fix the problems with `byval` parameters on that target and now correctly adds the `align` attribute. The problem is summarized in [this comment] by @eddyb. Briefly, 32-bit x86 has special alignment rules for `byval` parameters: for the most part, their alignment is forced to 4. This is not well-documented anywhere but in the Clang source. I looked at the logic in Clang `TargetInfo.cpp` and tried to replicate it here. The relevant methods in that file are `X86_32ABIInfo::getIndirectResult()` and `X86_32ABIInfo::getTypeStackAlignInBytes()`. The `align` parameter attribute for `byval` parameters in LLVM must match the platform ABI, or miscompilations will occur. Note that this doesn't use the approach suggested by eddyb, because I felt it was overkill to store the alignment in `on_stack` when special handling is really only needed for 32-bit x86. As a side effect, this should fix #80127, because it will make the `align` parameter attribute for `byval` parameters match the platform ABI on LLVM x86-64. [this comment]: https://github.com/rust-lang/rust/pull/80822#issuecomment-829985417 2023-07-08T13:38:40+00:00 Camille GILLOT gillot.camille@gmail.com 2023-05-15T19:15:28+00:00 urn:sha1:d7983a2f231a279984fc70eb428b936930eaa45c 2023-02-18T19:47:34+00:00 Michael Goulet michael@errs.io 2023-02-15T03:42:45+00:00 urn:sha1:e82cc656c822af7f1905b757a27cac5823fbc301