Skip to content

Conversation

durin42
Copy link
Contributor

@durin42 durin42 commented Mar 18, 2021

This lets LLVM be built using 2b5f3f4, which is only a few weeks old. The next change in LLVM (5de2d18) breaks rustc again by removing a function that's exposed into the Rust code, but I'll file a bug about that separately.

Please scrutinize the thinLTOResolvePrevailingInIndex call, as I'm not at all sure an empty config is right.

I'm also suspicious that a specific alignment could be specified in the call to CreateAtomicCmpXchg, but I don't know enough to figure that out.

Thanks!

durin42 added 3 commits March 16, 2021 16:45
…vailingInIndex

This changed in 54fb3ca - I'm not
entirely sure it's correct that we're leaving config empty, but the one
case in LLVM that looked similar did that.
As far as I can tell what we've been getting is llvm::MaybeAlign(), so
just use that for now. This is required sometime after
24539f1.
LLVM change 5fbd1a3 modified this
function to want std::string instead of StringRef, which is easily done.
@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @estebank (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 18, 2021
@Manishearth
Copy link
Member

Related: #83261 , which is preventing LLVM being updated past this point.

@tmandry
Copy link
Member

tmandry commented Mar 18, 2021

r? @nikic

@rust-highfive rust-highfive assigned nikic and unassigned estebank Mar 18, 2021
@tmiasko
Copy link
Contributor

tmiasko commented Mar 18, 2021

This will have to be accompanied by a LLVM submodule bump to a point where those changes can compile successfully, https://rustc-dev-guide.rust-lang.org/backend/updating-llvm.html.

@durin42
Copy link
Contributor Author

durin42 commented Mar 18, 2021

Right, I have that done locally, but it wasn't clear to me if I should include that in my PR. Should I put that commit on the end of the stack?

@nikic
Copy link
Contributor

nikic commented Mar 18, 2021

What's the background here? Is this just to allow local testing with a newer LLVM?

@durin42
Copy link
Contributor Author

durin42 commented Mar 19, 2021

Yeah, the background is other teams are using LLVM HEAD, so we're curious to explore using Rust against the same very-recent LLVM HEAD revision.

@durin42
Copy link
Contributor Author

durin42 commented Mar 22, 2021

Updated, PTAL

@nagisa
Copy link
Member

nagisa commented Mar 22, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 22, 2021

📌 Commit 9431e85 has been approved by nagisa

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 22, 2021
@cuviper
Copy link
Member

cuviper commented Mar 22, 2021

@tmiasko

This will have to be accompanied by a LLVM submodule bump

We generally do not update to arbitrary snapshots of LLVM development anymore, preferring to stick to LLVM release branches. We can still add compatibility for newer though, just as we also support a couple releases back.

@bors
Copy link
Collaborator

bors commented Mar 23, 2021

⌛ Testing commit 9431e85 with merge 4eb0bc7...

@bors
Copy link
Collaborator

bors commented Mar 23, 2021

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 4eb0bc7 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 23, 2021
@bors bors merged commit 4eb0bc7 into rust-lang:master Mar 23, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.