Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Draft: Upgrade to reuse 4.0.3 #5

Closed
wants to merge 3 commits into from
Closed

Draft: Upgrade to reuse 4.0.3 #5

wants to merge 3 commits into from

Conversation

jonathanpallant
Copy link

  • Updated requirements file
  • Ran reuse convert-dep5 to switch to new file format

No further changes were made to the new REUSE.toml file - I suspect some clean-ups will be required.

I verified that COPYRIGHT.md continues to build, but it has different contents.

@jonathanpallant
Upgrade to reuse 4.0.3
1d9a563 
) Updated requirements file
) Ran `reuse convert-dep5` to switch to new file format
pietroalbini
pietroalbini reviewed Jul 18, 2024
View reviewed changes
Copy link
Member

@pietroalbini pietroalbini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but we should evaluate whether to change precedence = "aggregate", since that is most likely wrong.

REUSE.toml Outdated Show resolved Hide resolved
@jonathanpallant
Copy link
Author

I'll read up on precedence, and make the TOML look nice.

@jonathanpallant
Copy link
Author

jonathanpallant commented Jul 18, 2024 •
edited
Loading

Precedence:

  • closest, the default value when precedence is not defined. This is an instruction to associate the Licensing Information inside of the Covered Files (or its adjacent .license file), if available. If no such Licensing Information is found, then the Licensing Information inside the table of the closest REUSE.toml that covers the File is associated. This algorithm is applied separately for copyright and for licensing. If a table for the same File in a closer REUSE.toml file has the override precedence, then that precedence is applied, and closest is ignored. This is effectively a fallback.
  • aggregate. This is an instruction to always associate the Licensing Information defined in the table with the table’s Covered Files. Subsequently, the closest logic is also applied.
  • override. This is an instruction to associate the Licensing Information defined in the table with the table’s Covered Files, and to ignore any other Licensing Information that is closer to the Files. The table in the REUSE.toml that is the closest to the root of the Project is authoritative.

Previously we used reuse-tool 1.1.0 in CI, which implemented REUSE 3.0. That did not feature a precedence option. Also, that version wouldn't actually work and we got an error saying:

Note that Rust requires some REUSE features that might not be present in the
release you're using. Make sure your REUSE release includes these PRs:

 - https://github.com/fsfe/reuse-tool/pull/623

If you use reuse-tool 2.0, you get errors like:

Copyright and licensing information for 'src/llvm-project/mlir/lib/Analysis/Liveness.cpp' have been found in 'src/llvm-project/mlir/lib/Analysis/Liveness.cpp' and the DEP5 file located at '.reuse/dep5'. The information in the DEP5 file has been overridden. Please ensure that this is correct.

I assume therefore that the original plan was to have the DEP5 file override the contents of the file, and so override seems like the correct answer here - although that requires we maintain the REUSE.toml file carefully. Perhaps only the LLVM sub-tree should be override?

As a test, I set everything to override and the COPYRIGHT.md file was unchanged.

Edit: I do actually have to re-install reuse 4.0 :)

@jonathanpallant
Copy link
Author

OK, aggregate is a bad choice, because reuse-tool doesn't really understand source code.

> > > **`peek-poke-derive`**  
> > > License: `Apache-2.0 OR MIT`  
> > > Copyright: The Rust Project Developers (see https://thanks.rust-lang.org)
> > > 
> > > *Exceptions:*
> > > 
> > > > **`LICENSE-APACHE`**  
> > > > License: `Apache-2.0 OR MIT`  
> > > > Copyright: License. Subject to the terms and conditions of  
> > > > Copyright: The Rust Project Developers (see https://thanks.rust-lang.org)

I'll set it to override.

@jonathanpallant
Copy link
Author

Opened rust-lang#127923

jonathanpallant pushed a commit that referenced this pull request Oct 18, 2024
@bors
Auto merge of rust-lang#131460 - jwong101:default-placement-new, r=ib…
1f67a7a 
…raheemdev

Optimize `Box::default` and `Arc::default` to construct more types in place

Both the `Arc` and `Box` `Default` impls currently call `T::default()` before allocating, and then moving the resulting `T` into the allocation.

Most `Default` impls are trivial, which should in theory allow
LLVM to construct `T: Default` directly in the `Box` allocation when calling
`<Box<T>>::default()`.

However, the allocation may fail, which necessitates calling `T`'s destructor if it has one.
If the destructor is non-trivial, then LLVM has a hard time proving that it's
sound to elide, which makes it construct `T` on the stack first, and then copy it into the allocation.

Change both of these impls to allocate first, and then call `T::default` into the uninitialized allocation, so that LLVM doesn't have to prove that it's sound to elide the destructor/initial stack copy.

For example, given the following Rust code:

```rust
#[derive(Default, Clone)]
struct Foo {
    x: Vec<u8>,
    z: String,
    y: Vec<u8>,
}

#[no_mangle]
pub fn src() -> Box<Foo> {
    Box::default()
}
```

<details open>
<summary>Before this PR:</summary>

```llvm
`@__rust_no_alloc_shim_is_unstable` = external global i8

; drop_in_place() generated in case the allocation fails

; core::ptr::drop_in_place<playground::Foo>
; Function Attrs: nounwind nonlazybind uwtable
define internal fastcc void `@"_ZN4core3ptr36drop_in_place$LT$playground..Foo$GT$17hff376aece491233bE"(ptr` noalias nocapture noundef readonly align 8 dereferenceable(72) %_1) unnamed_addr #0 personality ptr `@rust_eh_personality` {
start:
  %_1.val = load i64, ptr %_1, align 8
  %0 = icmp eq i64 %_1.val, 0
  br i1 %0, label %bb6, label %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i"

"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i": ; preds = %start
  %1 = getelementptr inbounds i8, ptr %_1, i64 8
  %_1.val6 = load ptr, ptr %1, align 8, !nonnull !3, !noundef !3
  tail call void `@__rust_dealloc(ptr` noundef nonnull %_1.val6, i64 noundef %_1.val, i64 noundef 1) rust-lang#8
  br label %bb6

bb6:                                              ; preds = %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i", %start
  %2 = getelementptr inbounds i8, ptr %_1, i64 24
  %.val9 = load i64, ptr %2, align 8
  %3 = icmp eq i64 %.val9, 0
  br i1 %3, label %bb5, label %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i.i11"

"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i.i11": ; preds = %bb6
  %4 = getelementptr inbounds i8, ptr %_1, i64 32
  %.val10 = load ptr, ptr %4, align 8, !nonnull !3, !noundef !3
  tail call void `@__rust_dealloc(ptr` noundef nonnull %.val10, i64 noundef %.val9, i64 noundef 1) rust-lang#8
  br label %bb5

bb5:                                              ; preds = %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i.i11", %bb6
  %5 = getelementptr inbounds i8, ptr %_1, i64 48
  %.val4 = load i64, ptr %5, align 8
  %6 = icmp eq i64 %.val4, 0
  br i1 %6, label %"_ZN4core3ptr46drop_in_place$LT$alloc..vec..Vec$LT$u8$GT$$GT$17hb5ca95423e113cf7E.exit16", label %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i15"

"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i15": ; preds = %bb5
  %7 = getelementptr inbounds i8, ptr %_1, i64 56
  %.val5 = load ptr, ptr %7, align 8, !nonnull !3, !noundef !3
  tail call void `@__rust_dealloc(ptr` noundef nonnull %.val5, i64 noundef %.val4, i64 noundef 1) rust-lang#8
  br label %"_ZN4core3ptr46drop_in_place$LT$alloc..vec..Vec$LT$u8$GT$$GT$17hb5ca95423e113cf7E.exit16"

"_ZN4core3ptr46drop_in_place$LT$alloc..vec..Vec$LT$u8$GT$$GT$17hb5ca95423e113cf7E.exit16": ; preds = %bb5, %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i15"
  ret void
}

; Function Attrs: nonlazybind uwtable
define noalias noundef nonnull align 8 ptr `@src()` unnamed_addr #1 personality ptr `@rust_eh_personality` {
start:

; alloca to place `Foo` in.
  %_1 = alloca [72 x i8], align 8
  call void `@llvm.lifetime.start.p0(i64` 72, ptr nonnull %_1)
  store i64 0, ptr %_1, align 8
  %_2.sroa.4.0._1.sroa_idx = getelementptr inbounds i8, ptr %_1, i64 8
  store ptr inttoptr (i64 1 to ptr), ptr %_2.sroa.4.0._1.sroa_idx, align 8
  %_2.sroa.5.0._1.sroa_idx = getelementptr inbounds i8, ptr %_1, i64 16
  %_3.sroa.4.0..sroa_idx = getelementptr inbounds i8, ptr %_1, i64 32
  call void `@llvm.memset.p0.i64(ptr` noundef nonnull align 8 dereferenceable(16) %_2.sroa.5.0._1.sroa_idx, i8 0, i64 16, i1 false)
  store ptr inttoptr (i64 1 to ptr), ptr %_3.sroa.4.0..sroa_idx, align 8
  %_3.sroa.5.0..sroa_idx = getelementptr inbounds i8, ptr %_1, i64 40
  %_4.sroa.4.0..sroa_idx = getelementptr inbounds i8, ptr %_1, i64 56
  call void `@llvm.memset.p0.i64(ptr` noundef nonnull align 8 dereferenceable(16) %_3.sroa.5.0..sroa_idx, i8 0, i64 16, i1 false)
  store ptr inttoptr (i64 1 to ptr), ptr %_4.sroa.4.0..sroa_idx, align 8
  %_4.sroa.5.0..sroa_idx = getelementptr inbounds i8, ptr %_1, i64 64
  store i64 0, ptr %_4.sroa.5.0..sroa_idx, align 8
  %0 = load volatile i8, ptr `@__rust_no_alloc_shim_is_unstable,` align 1, !noalias !4
  %_0.i.i.i = tail call noalias noundef align 8 dereferenceable_or_null(72) ptr `@__rust_alloc(i64` noundef 72, i64 noundef 8) rust-lang#8, !noalias !4
  %1 = icmp eq ptr %_0.i.i.i, null
  br i1 %1, label %bb2.i, label %"_ZN5alloc5boxed12Box$LT$T$GT$3new17h0864de14f863a27aE.exit"

bb2.i:                                            ; preds = %start
; invoke alloc::alloc::handle_alloc_error
  invoke void `@_ZN5alloc5alloc18handle_alloc_error17h98142d0d8d74161bE(i64` noundef 8, i64 noundef 72) rust-lang#9
          to label %.noexc unwind label %cleanup.i

.noexc:                                           ; preds = %bb2.i
  unreachable

cleanup.i:                                        ; preds = %bb2.i
  %2 = landingpad { ptr, i32 }
          cleanup
; call core::ptr::drop_in_place<playground::Foo>
  call fastcc void `@"_ZN4core3ptr36drop_in_place$LT$playground..Foo$GT$17hff376aece491233bE"(ptr` noalias noundef nonnull align 8 dereferenceable(72) %_1) rust-lang#10
  resume { ptr, i32 } %2

"_ZN5alloc5boxed12Box$LT$T$GT$3new17h0864de14f863a27aE.exit": ; preds = %start

; Copy from stack to heap if allocation is successful
  call void `@llvm.memcpy.p0.p0.i64(ptr` noundef nonnull align 8 dereferenceable(72) %_0.i.i.i, ptr noundef nonnull align 8 dereferenceable(72) %_1, i64 72, i1 false)
  call void `@llvm.lifetime.end.p0(i64` 72, ptr nonnull %_1)
  ret ptr %_0.i.i.i
}

```
</details>

<details>
<summary>After this PR</summary>

```llvm
; Notice how there's no `drop_in_place()` generated as well

define noalias noundef nonnull align 8 ptr `@src()` unnamed_addr #0 personality ptr `@rust_eh_personality` {
start:
; no stack allocation

  %0 = load volatile i8, ptr `@__rust_no_alloc_shim_is_unstable,` align 1
  %_0.i.i.i.i.i = tail call noalias noundef align 8 dereferenceable_or_null(72) ptr `@__rust_alloc(i64` noundef 72, i64 noundef 8) #5
  %1 = icmp eq ptr %_0.i.i.i.i.i, null
  br i1 %1, label %bb3.i, label %"_ZN5alloc5boxed16Box$LT$T$C$A$GT$13new_uninit_in17h80d6355ef4b73ea3E.exit"

bb3.i:                                            ; preds = %start
; call alloc::alloc::handle_alloc_error
  tail call void `@_ZN5alloc5alloc18handle_alloc_error17h98142d0d8d74161bE(i64` noundef 8, i64 noundef 72) rust-lang#6
  unreachable

"_ZN5alloc5boxed16Box$LT$T$C$A$GT$13new_uninit_in17h80d6355ef4b73ea3E.exit": ; preds = %start
; construct `Foo` directly into the allocation if successful

  store i64 0, ptr %_0.i.i.i.i.i, align 8
  %_8.sroa.4.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 8
  store ptr inttoptr (i64 1 to ptr), ptr %_8.sroa.4.0._1.sroa_idx, align 8
  %_8.sroa.5.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 16
  %_8.sroa.7.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 32
  tail call void `@llvm.memset.p0.i64(ptr` noundef nonnull align 8 dereferenceable(16) %_8.sroa.5.0._1.sroa_idx, i8 0, i64 16, i1 false)
  store ptr inttoptr (i64 1 to ptr), ptr %_8.sroa.7.0._1.sroa_idx, align 8
  %_8.sroa.8.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 40
  %_8.sroa.10.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 56
  tail call void `@llvm.memset.p0.i64(ptr` noundef nonnull align 8 dereferenceable(16) %_8.sroa.8.0._1.sroa_idx, i8 0, i64 16, i1 false)
  store ptr inttoptr (i64 1 to ptr), ptr %_8.sroa.10.0._1.sroa_idx, align 8
  %_8.sroa.11.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 64
  store i64 0, ptr %_8.sroa.11.0._1.sroa_idx, align 8
  ret ptr %_0.i.i.i.i.i
}
```

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pietroalbini pietroalbini pietroalbini approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jonathanpallant @pietroalbini