Clang powerpc64le pcrel code model is not used if -msoft-float or similar flags are used

[npiggin]

Example here - https://godbolt.org/z/1YhsEW7En

Remove -msoft-float or switch to GCC for expected pcrel code model. -mno-altivec and -mno-vsx flags at least also cause this. This prevents building the Linux kernel with the pcrel code model.

[llvmbot]

@llvm/issue-subscribers-backend-powerpc

[nemanjai]

Thanks for the report. I don't think there is a very good reason for PCRel to require hard float and altivec. I'll see if I can easily remove the requirement.

[stefanp-synopsys]

Took a look at this issue and it seems to change other flags as well. The parameter -msoft-float seems to clear a number of flags including Power 8 vector and Power 9 vector.
For example this test:

vector int testme() {
  vector int ret = {1, 2, 34, 52};
  return ret;
}

Can be compiled with:

clang -O3 -mcpu=pwr9 -S vectest.c

and produce:

addis 3, 2, .LCPI0_0@toc@ha
addi 3, 3, .LCPI0_0@toc@l
lxv 34, 0(3)
blr

If we compile the same code with the additional -msoft-float parameter we get:

	li 3, 1
	li 4, 2
	li 5, 34
	li 6, 52
	blr

I feel like that is not intended either.

[diggerlin]

the root cause of the problem , there is implication chain in the the features which defined in the PPC.td as

def FeaturePCRelativeMemops :
  SubtargetFeature<"pcrelative-memops", "HasPCRelativeMemops", "true",
                   "Enable PC relative Memory Ops",
                   [FeatureISA3_0, FeaturePrefixInstrs]>;

<-->

def FeaturePrefixInstrs : SubtargetFeature<"prefix-instrs", "HasPrefixInstrs",
                                           "true",
                                           "Enable prefixed instructions",
                                           [FeatureISA3_0, FeatureP8Vector,
                                            FeatureP9Altivec]>;

<-->

def FeatureP9Altivec : SubtargetFeature<"power9-altivec", "HasP9Altivec", "true",
                                        "Enable POWER9 Altivec instructions",
                                        [FeatureISA3_0, FeatureP8Altivec]>;

<-->

def FeatureP8Altivec : SubtargetFeature<"power8-altivec", "HasP8Altivec", "true",
                                        "Enable POWER8 Altivec instructions",
                                        [FeatureAltivec]>;

<-->

def FeatureAltivec   : SubtargetFeature<"altivec","HasAltivec", "true",
				"Enable Altivec instructions",
                                  [FeatureFPU]>;

<-->

def FeatureFPU       : SubtargetFeature<"fpu","HasFPU","true",
                                        "Enable classic FPU instructions",
                                        [FeatureHardFloat]>;

if we have the option -soft-float means FeatureFPU is false. it imply that FeaturePCRelativeMemops is false, it cause the powerpc64le pcrel code model is not sue when there is -msoft-float option.

there is a solution is to break the "FeatureAltivec" imply with FeatureFPU.

but there is code

static bool ppcUserFeaturesCheck(DiagnosticsEngine &Diags,
                                 const std::vector<std::string> &FeaturesVec) {
  // Cannot allow soft-float with Altivec.
  if (llvm::is_contained(FeaturesVec, "-hard-float") &&
      llvm::is_contained(FeaturesVec, "+altivec")) {
    Diags.Report(diag::err_opt_not_valid_with_opt) << "-msoft-float"
                                                   << "-maltivec";
    return false;
  }

  // Cannot allow soft-float with VSX.
  if (llvm::is_contained(FeaturesVec, "-hard-float") &&
      llvm::is_contained(FeaturesVec, "+vsx")) {
    Diags.Report(diag::err_opt_not_valid_with_opt) << "-msoft-float"
                                                   << "-mvsx";
    return false;
  }

[diggerlin]

I checked from git blame, the -msoft-float is not valid with +altivec is for the issue #55556 (the fix for the issue https://reviews.llvm.org/D126302)

[diggerlin]

Thanks for the report. I don't think there is a very good reason for PCRel to require hard float and altivec. I'll see if I can easily remove the requirement.

There are some PCRel instruction, for example : plxv (Prefixed Load VSX Vector 8LS:D-form) and pstxvp (Prefixed Store VSX Vector Paired 8LS:D-form) which is depend on the altivec

For plxv with R=1, let the effective address (EA) be the
sum of the address of the instruction and the value
d0||d1, sign-extended to 64 bits.

[diggerlin]

I think it is designed on purpose. do we need to fix the issue ? what is your opinion ? @chenzheng1030 , @nemanjai , @stefanp-ibm , @npiggin, @mandlebug

[chenzheng1030]

I think we need to change the design. It is understandable that altivec depends on hardware float point because when a system does not have hardware float point instructions, it is unlikely to have altivec instructions.

But I don't think soft float point should disable prefix instructions.

def FeaturePrefixInstrs : SubtargetFeature<"prefix-instrs", "HasPrefixInstrs",
                                           "true",
                                           "Enable prefixed instructions",
                                           [FeatureISA3_0, FeatureP8Vector,
                                            FeatureP9Altivec]>;

We may need to remove the FeatureP8Vector and FeatureP9Altivec subtarget feature. And in the instruction definition, when a prefixed instruction is a vector instruction, we need to add a P10vector predicate. Seems FeatureISA3_1 makes more sense than FeatureISA3_0 for FeaturePrefixInstrs?

[diggerlin]

Seems FeatureISA3_1 makes more sense than FeatureISA3_0 for FeaturePrefixInstrs

Yes, prefix related instructions are define is ISA3_1 ,for example: paddi , plxv etc