LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 33174 - Merge r298177 into the 4.0 branch : [X86] Add NumRegisterParameters Module Flag.
Summary: Merge r298177 into the 4.0 branch : [X86] Add NumRegisterParameters Module Flag.
Status: RESOLVED FIXED
Alias: None
Product: new-bugs
Classification: Unclassified
Component: new bugs (show other bugs)
Version: 4.0
Hardware: All All
: P enhancement
Assignee: Tom Stellard
URL: https://reviews.llvm.org/rL298177
Keywords:
Depends on:
Blocks: release-4.0.1
  Show dependency tree
 
Reported: 2017-05-25 13:26 PDT by Tom Stellard
Modified: 2017-05-31 07:01 PDT (History)
3 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Stellard 2017-05-25 13:26:08 PDT
Is this patch OK to merge to the 4.0 branch?
Comment 1 Richard Smith 2017-05-26 13:58:53 PDT
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.
Comment 2 Peter Collingbourne 2017-05-30 10:56:37 PDT
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.
Comment 3 Tom Stellard 2017-05-30 13:14:20 PDT
Merged: r304238
Comment 4 Tom Stellard 2017-05-30 13:18:29 PDT
Accidentally updated the wrong bug, r304238 is the merge commit from a different bug.
Comment 5 Tom Stellard 2017-05-31 02:59:37 PDT
Merged: r304294