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.

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

Already on GitHub? Sign in to your account

Upstream all the changes 馃檶 #187

Open
70 of 78 tasks
MaxDesiatov opened this issue Feb 17, 2020 · 5 comments
Open
70 of 78 tasks

Upstream all the changes 馃檶 #187

MaxDesiatov opened this issue Feb 17, 2020 · 5 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@MaxDesiatov
Copy link

MaxDesiatov commented Feb 17, 2020

This issue is for tracking the overall progress of upstreaming our changes, please post here if you create any new PRs with our changes in the upstream repository:

I think these capture only about a half of our changes, maybe much less, so more upstreaming effort is needed and welcome 馃檹

For upstreaming it's best to have a look at our umbrella PR apple#24684 and its diff and pick a self-contained and a relatively small patch that doesn't overlap or conflict with the rest of open PRs in the list above.

@MaxDesiatov MaxDesiatov added the good first issue Good for newcomers label Feb 17, 2020
@karwa
Copy link

karwa commented Mar 4, 2020

WRT the debugger tuning issue, it's possible this upstream patch might fix it: https://reviews.llvm.org/rG2504f14a06872f2e1755a88b3aab7e6bc280bec7

@MaxDesiatov MaxDesiatov changed the title Upstream all the changes to apple/swift 馃檶 Upstream all the changes 馃檶 May 9, 2020
@MaxDesiatov MaxDesiatov pinned this issue May 9, 2020
@MaxDesiatov
Copy link
Author

MaxDesiatov commented May 16, 2020

That upstream patch did not come through. I'd appreciate it if anyone reading this pushed it forward 馃檹 I'm not an expert in that area myself.

@karwa
Copy link

karwa commented May 17, 2020

@MaxDesiatov do you mean the debugging thing? I don鈥檛 know why the LLVM review thing says 鈥渁udit required鈥, but it appears that it did land: apple/llvm-project@2504f14

Also, I don鈥檛 think we need to set the thread mode any more. LLVM will lower atomic operations to regular loads/stores by itself: https://reviews.llvm.org/D58742

So that patch can just be dropped entirely, I think.

@ephemer
Copy link

ephemer commented Feb 11, 2021

FYI [LLVM] Support single-floating-point immediate value https://reviews.llvm.org/D77384. Status: been waiting for review for a month. has landed. Looks like that's the only one in the list that hasn't been updated yet.

Maybe this is the wrong place to ask but how many of these tasks are critical for things to "just work" in SwiftWASM? As someone unfamiliar with the requirements of getting feature parity I find it hard to know which of the unmerged pieces need merging, and as such what is the most important / urgent subtask here. Or is this essentially a list of places where the SwiftWasm fork differs from the Apple repo, meaning the SwiftWasm fork has to be maintained separately until they are merged?

@MaxDesiatov
Copy link
Author

MaxDesiatov commented Feb 11, 2021

how many of these tasks are critical for things to "just work"

A lot of the things already work pretty well in our fork, or do you have some specific criteria for "just work" here?

Or is this essentially a list of places where the SwiftWasm fork differs from the Apple repo, meaning the SwiftWasm fork has to be maintained separately until they are merged?

Yes, that's the primary purpose of this issue. We have an "umbrella" apple#24684 PR, which stays open as an easy way to see the toolchain diff. There's no plan to merge or push that PR as is, we're only taking small chunks from it and submit as separate PRs upstream, and those get added to the list here. Important to note that not everything from that diff has a corresponding upstream PR yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants