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

backport patch from #582 to LLVM8 (v2) #619

Closed
wants to merge 6 commits into from
Closed

backport patch from #582 to LLVM8 (v2) #619

wants to merge 6 commits into from

Conversation

IanButterworth
Copy link
Member

Fixes rebase. Replaces #596

cc. @vchuravy

@vchuravy
Copy link
Member

vchuravy commented Mar 14, 2020 •
edited
Loading

Thanks! Did you check that this fixes your test case and that the LLVM tests still pass?

@IanButterworth
Copy link
Member Author

The only thing I did was make the patching pass. It looks like all platforms are failing for a reason that’s not clear to me.


[03:34:10] ninja: build stopped: subcommand failed.
[03:34:10]  ---> ninja -j${nproc} -vv

@IanButterworth
Copy link
Member Author

If it's helpful to view the new patch with more context, here it is isolated IanButterworth/llvm-project@e0161f8

@IanButterworth IanButterworth force-pushed the ib/llvmpatch2 branch 2 times, most recently from 81267e4 to 3a8276e Compare March 15, 2020 04:37
@IanButterworth IanButterworth mentioned this pull request Mar 15, 2020
Merged
@vchuravy vchuravy requested a review from Keno March 17, 2020 15:19
IanButterworth
IanButterworth commented Mar 17, 2020
View reviewed changes
L/LLVM/LLVM@8.0.1/bundled/llvm_patches/0032-llvm-D75072-SCEV-add-type.patch Outdated
+ }

/// Return true if the backedge taken count is either the value returned by
/// getConstantMaxBackedgeTakenCount or zero.
Copy link
Member Author

Choose a reason for hiding this comment

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

I dropped these 3 during the rebase because they were just whitespace changes.

@staticfloat
Copy link
Member

Just FYI because I just merged the LLVM reorg, this will need to be updated accordingly. Patches and whatnot should live in the LLVM_full directories.

@IanButterworth
Copy link
Member Author

What broke?

@staticfloat
Copy link
Member

This looks like my fault. Just a minute.

@staticfloat
Fix LLVM_full build
0459f61 
We needed to return `platforms` from `configure_build()` as well.
@IanButterworth
Copy link
Member Author

@Keno. Ping for review. It’d be great to merge this

@IanButterworth IanButterworth mentioned this pull request Mar 23, 2020
Closed
@vchuravy
Copy link
Member

Thanks Ian I am going to shephard this through in #658

@vchuravy vchuravy closed this Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Keno Keno Awaiting requested review from Keno

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

Successfully merging this pull request may close these issues.
4 participants
@IanButterworth @vchuravy @staticfloat @ararslan