[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?

lib/CodeGen/BackendUtil.cpp

assert((CodeGenOpts.FloatABI == "soft" || CodeGenOpts.FloatABI == "softfp" ||
          "Invalid Floating Point ABI!");
   Options.FloatABIType =
       llvm::StringSwitch<llvm::FloatABI::ABIType>(CodeGenOpts.FloatABI)
           .Case("soft", llvm::FloatABI::Soft)
           .Case("softfp", llvm::FloatABI::Soft)
           .Case("hard", llvm::FloatABI::Hard)
           .Default(llvm::FloatABI::Default);
 
+  Options.RegParm = CodeGenOpts.NumRegisterParameters;
+
   // Set FP fusion mode.
   switch (CodeGenOpts.getFPContractMode()) {
   case CodeGenOptions::FPC_Off:
     Options.AllowFPOpFusion = llvm::FPOpFusion::Strict;
     break;
   case CodeGenOptions::FPC_On:
     Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
     break;