[PATCH] D67981: [NFC][PowerPC] Adding FeatureFPU in the definition of FeatureISA3_0

Mon Sep 30 08:04:24 PDT 2019

jsji added inline comments.

================
Comment at: llvm/lib/Target/PowerPC/PPC.td:193
+                                     "Enable instructions added in ISA 3.0.",
+                                     [FeatureFPU]>;
 def FeatureP9Altivec : SubtargetFeature<"power9-altivec", "HasP9Altivec", "true",
----------------
nemanjai wrote:
> jsji wrote:
> > jsji wrote:
> > > Looks like there is some misunderstanding of what `FeatureISA3_0` is for?
> > > 
> > > As in the description, `FeatureISA3_0` is for new instruction added in ISA 3.0, not all the instructions in ISA 3.0.
> > > Why it need to depend on FeatureFPU?
> > > 
> > > In other words, with the new patch `-mattr=-fpu` will also remove `isa-v30-instructions` effectively.
> > > So that means, if you add -mattr=-fpu, you can't use any of the new instruction added for P8? 
> > > Is that what you intended?
> > > 
> > I meant `P9`.
> Yes. That is very much part of the intent here. It is not possible to have a CPU that conforms to ISA 3.0 that does not support HW floating point operations (i.e. doesn't have a HW FPU).
> 
> Having an FPU does not imply we have an implementation of ISA 3.0. But having an implementation of ISA 3.0 **absolutely** implies having an FPU.
Yes, you are right, and I also agree that ` having an implementation of ISA 3.0 absolutely implies having an FPU`. 

However, looks like to me that `FeatureISA3_0` is NOT corresponding to `compliant implementation of ISA 3.0`.
But rather controlling whether we can generate *new instructions in ISA 3.0*.
Also, `FeatureFPU` is also not corresponding to `having an FPU`, but rather controlling whether we can generates FPU instructions.

So if we add the *Implies* to `FPU` here, we are more or less saying: all ISA 3.0 instructions are also FPU instructions,
we can only generate ISA 3.0 instructions if we are allow to generate FPU instructions.

Does this dependency really exists?

What if someone would like to avoid generating FPU instructions (eg: kernel users), 
but still would like to take advantage of P9 new instructions like `extswsli`, `maddld`,`modud`. 

I believe he/she can use `-mcpu=pwr9 -mattr=-fpu` right now, but will get performance deg if we apply this patch.

What do you think?

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67981/new/

https://reviews.llvm.org/D67981