[PowerPC] Fix the Predicates for enabling pcrelative-memops and PLXVP/PSTXVP definitions

[NeHuang]

Summary

In this patch, Predicates fix added to

• disable prefix-instrs will disable pcrelative-memops (pcrelative-memops imply prefix-instrs)
• set two predicates PairedVectorMemops and PrefixInstrs for PLXVP/PSTXVP definitions. (previously only guarded by PrefixInstrs)

[amyk]

I think it overall LGTM.

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1412

Question: is the hasSideEffects necessary?

[steven.zhang]

Test is missing. Please add a test to turn off the prefix instr to see if PLXVP disappear. And we have code like this, please double confirm if it is guarded by proper feature.

// If we encounter an LXVP/STXVP with an offset that doesn't fit, we can
// transform it to the prefixed version so we don't have to use the XForm.
if ((OpC == PPC::LXVP || OpC == PPC::STXVP) &&
    (!isInt<16>(Offset) || (Offset % offsetMinAlign(MI)) != 0)) {
  unsigned NewOpc = OpC == PPC::LXVP ? PPC::PLXVP : PPC::PSTXVP;
  MI.setDesc(TII.get(NewOpc));
  OpC = NewOpc;
}

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1412

Yes, it is needed as load/store instr also may have the sideeffects.

[amyk]

Agreed that a test should be added.

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1412

Right. Thanks for clarifying.

[NeHuang]

Add a test to disable prefix-instrs.

[steven.zhang]

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1412

I don't think there is any semantics difference with this change except that, we clear the sideeffect bit for PLXVP, which is not relative with what you want to do.

[NeHuang]

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1412

Agree. Removed hasSideEffects.

[steven.zhang]

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1412

So, do we need to change these lines any more?

[steven.zhang]

LGTM now.

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1412

Well, it is needed as the different predicates. In fact, what we need to do is just to change the predicates. But it is up to you.

llvm/lib/Target/PowerPC/PPC.td

 def FeaturePrefixInstrs : SubtargetFeature<"prefix-instrs", "HasPrefixInstrs",
                                            "true",
                                            "Enable prefixed instructions",
                                            [FeatureISA3_0, FeatureP8Vector,
                                             FeatureP9Altivec]>;
 def FeaturePCRelativeMemops :
   SubtargetFeature<"pcrelative-memops", "HasPCRelativeMemops", "true",
                    "Enable PC relative Memory Ops",
-                   [FeatureISA3_0]>;
+                   [FeatureISA3_0, FeaturePrefixInstrs]>;
 def FeaturePairedVectorMemops:
   SubtargetFeature<"paired-vector-memops", "PairedVectorMemops", "true",
                    "32Byte load and store instructions",
                    [FeatureISA3_0]>;
 def FeatureMMA : SubtargetFeature<"mma", "HasMMA", "true",
                                   "Enable MMA instructions",
                                   [FeatureP8Vector, FeatureP9Altivec,
                                    FeaturePairedVectorMemops]>;

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

 
 let mayLoad = 1, mayStore = 0, Predicates = [PairedVectorMemops] in {
   def LXVP : DQForm_XTp5_RA17_MEM<6, 0, (outs vsrprc:$XTp),
                                   (ins memrix16:$DQ_RA), "lxvp $XTp, $DQ_RA",
                                   IIC_LdStLFD, []>;
   def LXVPX : XForm_XTp5_XAB5<31, 333, (outs vsrprc:$XTp), (ins memrr:$src),
                               "lxvpx $XTp, $src", IIC_LdStLFD,
                               []>;
-  let Predicates = [PrefixInstrs] in {
-    defm PLXVP :
-      8LS_DForm_R_XTp5_SI34_MEM_p<1, 58, (outs vsrprc:$XTp), (ins memri34:$D_RA),
-                                  (ins memri34_pcrel:$D_RA), "plxvp $XTp, $D_RA",
-                                  IIC_LdStLFD>;
-  }
 }
 
 let mayLoad = 0, mayStore = 1, Predicates = [PairedVectorMemops] in {
   def STXVP : DQForm_XTp5_RA17_MEM<6, 1, (outs), (ins vsrprc:$XTp,
                                    memrix16:$DQ_RA), "stxvp $XTp, $DQ_RA",
                                    IIC_LdStLFD, []>;
   def STXVPX : XForm_XTp5_XAB5<31, 461, (outs), (ins vsrprc:$XTp, memrr:$dst),
                                "stxvpx $XTp, $dst", IIC_LdStLFD,
                                []>;
-  let Predicates = [PrefixInstrs] in {
-    defm PSTXVP :
-      8LS_DForm_R_XTp5_SI34_MEM_p<1, 62, (outs), (ins vsrprc:$XTp, memri34:$D_RA),
-                                  (ins vsrprc:$XTp, memri34_pcrel:$D_RA),
-                                  "pstxvp $XTp, $D_RA", IIC_LdStLFD>;
-  }
-}
+}
+
+let mayLoad = 1, mayStore = 0, Predicates = [PairedVectorMemops, PrefixInstrs] in {
+  defm PLXVP :
+    8LS_DForm_R_XTp5_SI34_MEM_p<1, 58, (outs vsrprc:$XTp), (ins memri34:$D_RA),
+                                (ins memri34_pcrel:$D_RA), "plxvp $XTp, $D_RA",
+                                IIC_LdStLFD>;
+}
+
+let mayLoad = 0, mayStore = 1, Predicates = [PairedVectorMemops, PrefixInstrs] in {
 
 // TODO: We have an added complexity of 500 here. This is only a temporary
 // solution to have tablegen consider these patterns first. The way we do
 // addressing for PowerPC is complex depending on available D form, X form, or
 // aligned D form loads/stores like DS and DQ forms. The prefixed
 // instructions in this file also add additional PC Relative loads/stores
 // and D form loads/stores with 34 bit immediates. It is very difficult to force
 // instruction selection to consistently pick these first without the current

llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -O2 \
 ; RUN:     -ppc-asm-full-reg-names -mcpu=pwr10 < %s | FileCheck %s
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -O2 \
 ; RUN:     -ppc-asm-full-reg-names -mcpu=pwr10 < %s | FileCheck %s \
 ; RUN:     --check-prefix=CHECK-NOPCREL
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -O2 \
 ; RUN:     -mattr=-pcrelative-memops -ppc-asm-full-reg-names -mcpu=pwr10 < %s | \
 ; RUN:     FileCheck %s --check-prefix=CHECK-NOPCREL
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -O2 \
+; RUN:     -mattr=-prefix-instrs -ppc-asm-full-reg-names -mcpu=pwr10 < %s | \
+; RUN:     FileCheck %s --check-prefix=CHECK-NOPCREL
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -O2 \
 ; RUN:     -ppc-asm-full-reg-names -target-abi=elfv2 -mcpu=pwr10 < %s | \
 ; RUN:     FileCheck %s
 
 define dso_local <2 x double> @testDoubleToDoubleFail() local_unnamed_addr {
 ; CHECK-LABEL: testDoubleToDoubleFail:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    plxv vs34, .LCPI0_0@PCREL(0), 1