[PATCH] D27051: [X86] Add NumRegisterParameters Module Flag

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: lib/CodeGen/CodeGenModule.cpp:421
+  // Record mregparm value.
+  getModule().addModuleFlag(llvm::Module::Error, "NumRegisterParameters",
+                            CodeGenOpts.NumRegisterParameters);
----------------
niravd wrote:
> rnk wrote:
> > I'd like this to be conditional on NumRegisterParameters being non-zero, so that it doesn't pollute the vast majority of modules that don't use -mregparm. I think you will get the right LTO diagnostic behavior if you use llvm::Module::Require instead of Error here.
> My understanding is that Require only checks the post-linking values, so we'd not error when linking regparm 0 with regparm N. 
> 
> That said, it'd be easy enough to restrict emitting the module flag to target that actually depend on it, i.e. X86-32. That would get us 90% of the way.
Hm, nevermind then. Let's go with what you have.


================
Comment at: test/CodeGen/pr3997.c:5
+
+void use_builtin_memcpy(void *dest, const void *src, unsigned int n) {
+  __builtin_memcpy(dest, src, n);
----------------
Probably worth checking that this calls llvm.memcpy without any inreg attributes.


================
Comment at: test/CodeGen/pr3997.c:10
+void use_memcpy(void *dest, const void *src, unsigned int n) {
+  memcpy(dest, src, n);
+}
----------------
Probably worth CHECKs for whatever we do for this today.


https://reviews.llvm.org/D27051