[X86] Add NumRegisterParameters Module Flag

[mkuper]

So, I remember back when I implemented the MCU ABI rnk and I discussed this, and didn't really reach a definitive conclusion.

This would make -mregparm behavior match GCC, but would be an ABI break for clang.
You may want to surface this on llvm-dev, and check that doesn't come as a surprise to anyone.

[mkuper]

Another issue is that we need to serialize this as part of the IR for the function.
Since we don't use inreg attributes, it needs to either be a part of the calling convention (although we currently have no way to parametrize the CC, I think), or a function attribute.

[rnk]

This seems like a good use case for module flags, actually. Check out Module::ModFlagBehavior::Error in particular.

[mkuper]

I'm not sure. It basically comes down to - do we want to ever be able to combine (say, link) -mregparm code with non-mregparm code.

For the MCU this was a non-issue, because the answer is always no.

[niravd]

Change propagate to use subtarget features

[niravd]

Change to llvm module flag

[jlebar]

test/CodeGenCUDA/flush-denormals.cu

21 ↗

(On Diff #85928)

Hm, this is a bummer, but I don't immediately see a better way to do it.

[rnk]

lib/CodeGen/CodeGenModule.cpp

421 ↗

(On Diff #85928)

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.

test/CodeGenCUDA/flush-denormals.cu

21 ↗

(On Diff #85928)

I think this can be removed if the module flag is made conditional.

[niravd]

lib/CodeGen/CodeGenModule.cpp

421 ↗

(On Diff #85928)

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.

[rnk]

lgtm

lib/CodeGen/CodeGenModule.cpp

421 ↗

(On Diff #85928)

Hm, nevermind then. Let's go with what you have.

test/CodeGen/pr3997.c

5 ↗

(On Diff #85928)

Probably worth checking that this calls llvm.memcpy without any inreg attributes.

10 ↗

(On Diff #85928)

Probably worth CHECKs for whatever we do for this today.

[yaron.keren]

Most other module flags are added at CodeGenModule::Release(). For consistency, could this code be in CodeGenModule::Release() as well?

[yaron.keren]

No intention of any major work intended... just moved the code (locally) to Release and regression tests were still passing.

I must be missing something - isn't the module flag is used in X86ISelLowering which is called after CodeGenModule::Release() is done?

cfe/trunk/lib/CodeGen/CodeGenModule.cpp

if (CodeGenOpts.hasProfileClangUse()) {
     } else
       PGOReader = std::move(ReaderOrErr.get());
   }
 
   // If coverage mapping generation is enabled, create the
   // CoverageMappingModuleGen object.
   if (CodeGenOpts.CoverageMapping)
     CoverageMapping.reset(new CoverageMappingModuleGen(*this, *CoverageInfo));
+
+  // Record mregparm value now so it is visible through rest of codegen.
+  if (Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86)
+    getModule().addModuleFlag(llvm::Module::Error, "NumRegisterParameters",
+                              CodeGenOpts.NumRegisterParameters);
+
 }
 
 CodeGenModule::~CodeGenModule() {}
 
 void CodeGenModule::createObjCRuntime() {
   // This is just isGNUFamily(), but we want to force implementors of
   // new ABIs to decide how best to do this.
   switch (LangOpts.ObjCRuntime.getKind()) {
void CodeGenModule::Release() {
   emitLLVMUsed();
   if (SanStats)
     SanStats->finish();
 
   if (CodeGenOpts.Autolink &&
       (Context.getLangOpts().Modules || !LinkerOptionsMetadata.empty())) {
     EmitModuleLinkOptions();
   }
+
   if (CodeGenOpts.DwarfVersion) {
     // We actually want the latest version when there are conflicts.
     // We can change from Warning to Latest if such mode is supported.
     getModule().addModuleFlag(llvm::Module::Warning, "Dwarf Version",
                               CodeGenOpts.DwarfVersion);
   }
   if (CodeGenOpts.EmitCodeView) {
     // Indicate that we want CodeView in the metadata.

cfe/trunk/test/CodeGen/pr3997.c

+// RUN: %clang_cc1 %s -triple i386-unknown-linux-gnu -mregparm 3 -emit-llvm -o - | FileCheck %s
+
+void *memcpy(void *dest, const void *src, unsigned int n);
+
+void use_builtin_memcpy(void *dest, const void *src, unsigned int n) {
+  __builtin_memcpy(dest, src, n);
+}
+
+void use_memcpy(void *dest, const void *src, unsigned int n) {
+  memcpy(dest, src, n);
+}
+
+//CHECK: !{i32 1, !"NumRegisterParameters", i32 3}

cfe/trunk/test/Headers/altivec-header.c

 // RUN: %clang_cc1 -triple powerpc64-unknown-unknown -faltivec -ffreestanding -emit-llvm -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple powerpc64-unknown-unknown -faltivec -ffreestanding -emit-llvm -fno-lax-vector-conversions -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple powerpc64-unknown-unknown -faltivec -ffreestanding -emit-llvm -x c++ -o - %s | FileCheck %s
 
 #include <altivec.h>
 
 // Verify that simply including <altivec.h> does not generate any code
 // (i.e. all inline routines in the header are marked "static")
 
 // CHECK: target triple = "powerpc64-
 // CHECK-NEXT: {{^$}}
-// CHECK-NEXT: llvm.ident
+// CHECK-NEXT: {{llvm\..*}}