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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge r298177 into the 4.0 branch : [X86] Add NumRegisterParameters Module Flag. #32521

Closed
tstellar opened this issue May 25, 2017 · 6 comments
Closed
Assignees
Labels
bugzilla Issues migrated from bugzilla

Comments

@tstellar
Copy link
Collaborator

Bugzilla Link 33174
Resolution FIXED
Resolved on May 31, 2017 07:01
Version 4.0
OS All
Blocks #31409
CC @pcc

Extended Description

Is this patch OK to merge to the 4.0 branch?

@tstellar
Copy link
Collaborator Author

assigned to @tstellar

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented May 26, 2017

Looks like this is an LLVM IR ABI break: if we merge this, you won't be able to LTO code from clang 4.0.0 with code from clang 4.0.1 if you use -mregparm.

On the other hand, if we don't merge this, then building with -mregparm at all will potentially miscompile. I'm inclined to say that's more important than maintaining LLVM IR compatibility between patch releases here. Mechanically, the patch looks OK to merge, assuming you're OK with its implications.

I believe you also need to merge r298179 for this patch to have any effect.

@pcc
Copy link
Contributor

pcc commented May 30, 2017

Seems fine to me.

Regarding LTO'ing code from 4.0.0 with code from 4.0.1: that should still work. Linking a module with a module flag against another module without that module flag will give you a module with the module flag from the first module. That means that the backend will use the -mregparm flag from the 4.0.1 module, which is probably the best that we can do.

@tstellar
Copy link
Collaborator Author

Merged: r304238

@tstellar
Copy link
Collaborator Author

Accidentally updated the wrong bug, r304238 is the merge commit from a different bug.

@tstellar
Copy link
Collaborator Author

Merged: r304294

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

2 participants