[PowerPC] Clear the sideeffect bit for those instructions that didn't have the match pattern

[steven.zhang]

Summary

If the instruction have match pattern, llvm-tblgen will infer the sideeffect bit from the match pattern and it works well. If not, the tblgen will set it as true that hurt the scheduling.

// The mayLoad and mayStore flags default to false.
// Conservatively assume hasSideEffects if it wasn't explicit.
if (InstInfo->hasSideEffects_Unset)
  InstInfo->hasSideEffects = true;

PowerPC has some instructions that didn't specify the match pattern(i.e. LXSD etc), which is manually selected post-ra according to the register pressure. We need to clear the sideeffect flag for these instructions.

[steven.zhang]

Gentle ping ...

[hfinkel]

LGTM

[jsji]

I don't see any connection of this bit to match pattern.
Can you explain why we want to limit that didn't have the match pattern -- We actually didn't care about the match pattern in your patch either.

Overall LGTM as well, but maybe we can improve the testcase further. Thanks.

llvm/test/CodeGen/PowerPC/mi-scheduling.ll

36 ↗

(On Diff #225814)

Global memory objects can be hasOrderedMemoryRef as well, so having Global memory object might NOT always due to hasUnmodeledSideEffects.
And it is *NOT* easy to come up the testcase in scheduling that can test the other affected opcode above.
so mi-scheduling might not be a great place to check hasUnmodeledSideEffects.

Maybe we can check it in peephole-opt with MIR. eg:

$ cat tm.mir
name:            sideeffects
alignment:       16
liveins:
  - { reg: '$x3', virtual-reg: '' }
body:             |
  bb.0.entry:
    liveins: $x3
   renamable $vf1 = LXSD 136, renamable $x3 ::

...

$ llc -run-pass peephole-opt tm.mir -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr9 -debug-only=peephole-opt
********** PEEPHOLE OPTIMIZER **********
********** Function: sideeffects
NAPhysCopy: blowing away all info due to renamable $vf1 = LXSD 136, renamable $x3
Encountered load fold barrier on renamable $vf1 = LXSD 136, renamable $x3

This way, we can easily add more affected opcodes in the MIR tests. What do you think?

[steven.zhang]

In D69232#1724229, @jsji wrote:

I don't see any connection of this bit to match pattern.
Can you explain why we want to limit that didn't have the match pattern -- We actually didn't care about the match pattern in your patch either.

Overall LGTM as well, but maybe we can improve the testcase further. Thanks.

If we didn't have the match pattern, llvm-tblgen will set the sideeffects as true if we didn't specify it. Otherwise, it will infer this flag from match pattern. And if we didn't set the sideeffect for match pattern(usually case), it will set it as false by default. So, what we need to clear is about the case that didn't have the match pattern.

llvm/test/CodeGen/PowerPC/mi-scheduling.ll

36 ↗

(On Diff #225814)

Your suggestion will definitely work. However, as I am checking NOT having "Global memory object" and it will cover the hasUnmodeledSideEffects and other conditions you mentioned. Personally, I want to check that, LXSD SU didn't have memory dependency with others, not just the sideeffects flag.

[jsji]

llvm/test/CodeGen/PowerPC/mi-scheduling.ll

36 ↗

(On Diff #225814)

Fair enough, but in that case, you shouldn't call the function: sideeffects, and the comments should be updated to reflect your test points.

Also this IR can be further reduced to include ONLY LXSD and fewer fadd, store is unnecessary.

And what is more, we should add more testing for other affected op? Eg: opcode with match patterns like 'LXSIBZX'? Make sure nothing changed?

[hfinkel]

llvm/test/CodeGen/PowerPC/mi-scheduling.ll

36 ↗

(On Diff #225814)

It's probably best if we just add a utility that dumps all of the instruction properties, and then we can directly test the output of that utility. I'd certainly welcome some follow-up patch which did that.

[steven.zhang]

llvm/test/CodeGen/PowerPC/mi-scheduling.ll

36 ↗

(On Diff #225814)

This is a good idea to verify the instruction properties as it is really needed. However, need some effort. I will create a follow up issue for this. Thank you.

36 ↗

(On Diff #225814)

re jsji: Good point. I will update the comments, rename the function name, and remove the store. For other opcode, I think, we need some follow up as hfinkel said to verify them together.

[steven.zhang]

Update the test and move it into the scheduling-mem-dependency.ll. And update the test in scheduling-mem-dependency.ll. to make it more robust together with this patch, as it was added by me before ...

[jsji]

llvm/test/CodeGen/PowerPC/mi-scheduling.ll

36 ↗

(On Diff #225814)

It's probably best if we just add a utility that dumps all of the instruction properties, and then we can directly test the output of that utility. I'd certainly welcome some follow-up patch which did that.

Yes, good idea for such utility, we should be able to catch quite some hidden bugs with it.

[hfinkel]

llvm/test/CodeGen/PowerPC/mi-scheduling.ll

36 ↗

(On Diff #225814)

Yes, good idea for such utility, we should be able to catch quite some hidden bugs with it.

Exactly; I used to do this every once in a while my manually searching through the TableGen-generated files; I suppose that we might be able to do this by literally running TableGen and matching the output, but a separate utility might be better from a build-process-management standpoint and because it won't require rerunning TableGen (which could be slow in Debug mode).

llvm/lib/Target/PowerPC/PPCInstr64Bit.td

def LHAX8: XForm_1_memOp<31, 343, (outs g8rc:$rD), (ins memrr:$src),
                         "lhax $rD, $src", IIC_LdStLHA,
                         [(set i64:$rD, (sextloadi16 xaddr:$src))]>,
                         PPC970_DGroup_Cracked;
 def LWAX : XForm_1_memOp<31, 341, (outs g8rc:$rD), (ins memrr:$src),
                         "lwax $rD, $src", IIC_LdStLHA,
                         [(set i64:$rD, (sextloadi32 xaddrX4:$src))]>, isPPC64,
                         PPC970_DGroup_Cracked;
 // For fast-isel:
-let isCodeGenOnly = 1, mayLoad = 1 in {
+let isCodeGenOnly = 1, mayLoad = 1, hasSideEffects = 0 in {
 def LWA_32  : DSForm_1<58, 2, (outs gprc:$rD), (ins memrix:$src),
                       "lwa $rD, $src", IIC_LdStLWA, []>, isPPC64,
                       PPC970_DGroup_Cracked;
 def LWAX_32 : XForm_1_memOp<31, 341, (outs gprc:$rD), (ins memrr:$src),
                             "lwax $rD, $src", IIC_LdStLHA, []>, isPPC64,
                             PPC970_DGroup_Cracked;
 } // end fast-isel isCodeGenOnly
 

llvm/lib/Target/PowerPC/PPCInstrInfo.td

def LFIWAX : XForm_25_memOp<31, 855, (outs f8rc:$frD), (ins memrr:$src),
                       [(set f64:$frD, (PPClfiwax xoaddr:$src))]>;
 def LFIWZX : XForm_25_memOp<31, 887, (outs f8rc:$frD), (ins memrr:$src),
                       "lfiwzx $frD, $src", IIC_LdStLFD,
                       [(set f64:$frD, (PPClfiwzx xoaddr:$src))]>;
 }
 }
 
 // Load Multiple
+let mayLoad = 1, mayStore = 0, hasSideEffects = 0 in 
 def LMW : DForm_1<46, (outs gprc:$rD), (ins memri:$src),
                   "lmw $rD, $src", IIC_LdStLMW, []>;
 
 //===----------------------------------------------------------------------===//
 // PPC32 Store Instructions.
 //
 
 // Unindexed (r+i) Stores.

 let Predicates = [HasFPU] in {
 def : Pat<(pre_store f32:$rS, iPTR:$ptrreg, iPTR:$ptroff),
           (STFSUX $rS, $ptrreg, $ptroff)>;
 def : Pat<(pre_store f64:$rS, iPTR:$ptrreg, iPTR:$ptroff),
           (STFDUX $rS, $ptrreg, $ptroff)>;
 }
 
 // Store Multiple
+let mayStore = 1, mayLoad = 0, hasSideEffects = 0 in
 def STMW : DForm_1<47, (outs), (ins gprc:$rS, memri:$dst),
                    "stmw $rS, $dst", IIC_LdStLMW, []>;
 
 def SYNC : XForm_24_sync<31, 598, (outs), (ins i32imm:$L),
                         "sync $L", IIC_LdStSync, []>;
 
 let isCodeGenOnly = 1 in {
   def MSYNC : XForm_24_sync<31, 598, (outs), (ins),

llvm/lib/Target/PowerPC/PPCInstrVSX.td

let AddedComplexity = 400 in { // Prefer VSX patterns over non-VSX patterns.
   }
 
   def XXLORC : XX3Form<60, 170,
                        (outs vsrc:$XT), (ins vsrc:$XA, vsrc:$XB),
                        "xxlorc $XT, $XA, $XB", IIC_VecGeneral,
                        [(set v4i32:$XT, (or v4i32:$XA, (vnot_ppc v4i32:$XB)))]>;
 
   // VSX scalar loads introduced in ISA 2.07
-  let mayLoad = 1, mayStore = 0 in {
+  let mayLoad = 1, mayStore = 0, hasSideEffects = 0 in {
     let CodeSize = 3 in
     def LXSSPX : XX1Form_memOp<31, 524, (outs vssrc:$XT), (ins memrr:$src),
                          "lxsspx $XT, $src", IIC_LdStLFD, []>;
     def LXSIWAX : XX1Form_memOp<31, 76, (outs vsfrc:$XT), (ins memrr:$src),
                           "lxsiwax $XT, $src", IIC_LdStLFD, []>;
     def LXSIWZX : XX1Form_memOp<31, 12, (outs vsfrc:$XT), (ins memrr:$src),
                           "lxsiwzx $XT, $src", IIC_LdStLFD, []>;
 
     // Pseudo instruction XFLOADf32 will be expanded to LXSSPX or LFSX later
     let CodeSize = 3 in
     def XFLOADf32  : PseudoXFormMemOp<(outs vssrc:$XT), (ins memrr:$src),
                             "#XFLOADf32",
                             [(set f32:$XT, (load xoaddr:$src))]>;
     // Pseudo instruction LIWAX will be expanded to LXSIWAX or LFIWAX later
     def LIWAX : PseudoXFormMemOp<(outs vsfrc:$XT), (ins memrr:$src),
                        "#LIWAX",
                        [(set f64:$XT, (PPClfiwax xoaddr:$src))]>;
     // Pseudo instruction LIWZX will be expanded to LXSIWZX or LFIWZX later
     def LIWZX : PseudoXFormMemOp<(outs vsfrc:$XT), (ins memrr:$src),
                        "#LIWZX",
                        [(set f64:$XT, (PPClfiwzx xoaddr:$src))]>;
   } // mayLoad
 
   // VSX scalar stores introduced in ISA 2.07
-  let mayStore = 1, mayLoad = 0 in {
+  let mayStore = 1, mayLoad = 0, hasSideEffects = 0 in {
     let CodeSize = 3 in
     def STXSSPX : XX1Form_memOp<31, 652, (outs), (ins vssrc:$XT, memrr:$dst),
                           "stxsspx $XT, $dst", IIC_LdStSTFD, []>;
     def STXSIWX : XX1Form_memOp<31, 140, (outs), (ins vsfrc:$XT, memrr:$dst),
                           "stxsiwx $XT, $dst", IIC_LdStSTFD, []>;
 
     // Pseudo instruction XFSTOREf32 will be expanded to STXSSPX or STFSX later
     let CodeSize = 3 in
let AddedComplexity = 400, Predicates = [HasP9Vector] in {
   def XXSPLTIB : X_RD6_IMM8<60, 360, (outs vsrc:$XT), (ins u8imm:$IMM8),
                             "xxspltib $XT, $IMM8", IIC_VecPerm, []>;
 
   //===--------------------------------------------------------------------===//
   // Vector/Scalar Load/Store Instructions
 
   // When adding new D-Form loads/stores, be sure to update the ImmToIdxMap in
   // PPCRegisterInfo::PPCRegisterInfo and maybe save yourself some debugging.
-  let mayLoad = 1, mayStore = 0 in {
+  let mayLoad = 1, mayStore = 0, hasSideEffects = 0 in {
   // Load Vector
   def LXV : DQ_RD6_RS5_DQ12<61, 1, (outs vsrc:$XT), (ins memrix16:$src),
                             "lxv $XT, $src", IIC_LdStLFD, []>;
   // Load DWord
   def LXSD  : DSForm_1<57, 2, (outs vfrc:$vD), (ins memrix:$src),
                        "lxsd $vD, $src", IIC_LdStLFD, []>;
   // Load SP from src, convert it to DP, and place in dword[0]
   def LXSSP : DSForm_1<57, 3, (outs vfrc:$vD), (ins memrix:$src),
def LXVLL : XX1Form_memOp<31,301, (outs vsrc:$XT), (ins memr:$src, g8rc:$rB),
                    [(set v4i32:$XT, (int_ppc_vsx_lxvll addr:$src, i64:$rB))]>;
 
   // Load Vector Word & Splat Indexed
   def LXVWSX  : X_XT6_RA5_RB5<31, 364, "lxvwsx" , vsrc, []>;
   } // mayLoad
 
   // When adding new D-Form loads/stores, be sure to update the ImmToIdxMap in
   // PPCRegisterInfo::PPCRegisterInfo and maybe save yourself some debugging.
-  let mayStore = 1, mayLoad = 0 in {
+  let mayStore = 1, mayLoad = 0, hasSideEffects = 0 in {
   // Store Vector
   def STXV : DQ_RD6_RS5_DQ12<61, 5, (outs), (ins vsrc:$XT, memrix16:$dst),
                              "stxv $XT, $dst", IIC_LdStSTFD, []>;
   // Store DWord
   def STXSD  : DSForm_1<61, 2, (outs), (ins vfrc:$vS, memrix:$dst),
                         "stxsd $vS, $dst", IIC_LdStSTFD, []>;
   // Convert DP of dword[0] to SP, and Store to dst
   def STXSSP : DSForm_1<61, 3, (outs), (ins vfrc:$vS, memrix:$dst),
def : Pat<(f128 (PPCbuild_fp128 i64:$rB, i64:$rA)),
               (f128 (COPY_TO_REGCLASS (MTVSRDD $rB, $rA), VRRC))>;
   }
   let Predicates = [IsISA3_0, HasP9Vector, HasDirectMove, IsLittleEndian] in {
     def : Pat<(f128 (PPCbuild_fp128 i64:$rA, i64:$rB)),
               (f128 (COPY_TO_REGCLASS (MTVSRDD $rB, $rA), VRRC))>;
   }
 }
 
-let Predicates = [HasP9Vector] in {
+let Predicates = [HasP9Vector], hasSideEffects = 0 in {
   let mayStore = 1 in {
     def SPILLTOVSR_STX : PseudoXFormMemOp<(outs),
                                           (ins spilltovsrrc:$XT, memrr:$dst),
                                           "#SPILLTOVSR_STX", []>;
     def SPILLTOVSR_ST : PPCPostRAExpPseudo<(outs), (ins spilltovsrrc:$XT, memrix:$dst),
                               "#SPILLTOVSR_ST", []>;
   }
   let mayLoad = 1 in {

llvm/test/CodeGen/PowerPC/extract-and-store.ll

 ; CHECK-BE-NEXT:    rldicl r6, r7, 56, 56
 ; CHECK-BE-NEXT:    stb r4, 9(r5)
 ; CHECK-BE-NEXT:    stb r3, 11(r5)
 ; CHECK-BE-NEXT:    stb r6, 12(r5)
 ; CHECK-BE-NEXT:    blr
 ;
 ; CHECK-P9-LABEL: test_13_consecutive_stores_of_bytes:
 ; CHECK-P9:       # %bb.0: # %entry
-; CHECK-P9-NEXT:    vsldoi v3, v2, v2, 4
-; CHECK-P9-NEXT:    stxsibx vs35, 0, r5
 ; CHECK-P9-NEXT:    vsldoi v3, v2, v2, 12
 ; CHECK-P9-NEXT:    li r3, 1
 ; CHECK-P9-NEXT:    stxsibx vs35, r5, r3
 ; CHECK-P9-NEXT:    vsldoi v3, v2, v2, 15
 ; CHECK-P9-NEXT:    li r3, 2
 ; CHECK-P9-NEXT:    stxsibx vs35, r5, r3
 ; CHECK-P9-NEXT:    vsldoi v3, v2, v2, 1
 ; CHECK-P9-NEXT:    li r3, 3
 ; CHECK-P9-NEXT:    stxsibx vs35, r5, r3
 ; CHECK-P9-NEXT:    vsldoi v3, v2, v2, 2
 ; CHECK-P9-NEXT:    li r3, 4
 ; CHECK-P9-NEXT:    stxsibx vs35, r5, r3
+; CHECK-P9-NEXT:    vsldoi v3, v2, v2, 4
+; CHECK-P9-NEXT:    stxsibx vs35, 0, r5
 ; CHECK-P9-NEXT:    vsldoi v3, v2, v2, 8
 ; CHECK-P9-NEXT:    li r3, 5
 ; CHECK-P9-NEXT:    stxsibx vs35, r5, r3
 ; CHECK-P9-NEXT:    vsldoi v3, v2, v2, 13
 ; CHECK-P9-NEXT:    li r3, 6
 ; CHECK-P9-NEXT:    stxsibx vs35, r5, r3
 ; CHECK-P9-NEXT:    vsldoi v3, v2, v2, 11
 ; CHECK-P9-NEXT:    li r3, 7

 ; CHECK-P9-NEXT:    vsldoi v2, v2, v2, 10
 ; CHECK-P9-NEXT:    stxsibx vs35, r5, r3
 ; CHECK-P9-NEXT:    li r3, 12
 ; CHECK-P9-NEXT:    stxsibx vs34, r5, r3
 ; CHECK-P9-NEXT:    blr
 ;
 ; CHECK-P9-BE-LABEL: test_13_consecutive_stores_of_bytes:
 ; CHECK-P9-BE:       # %bb.0: # %entry
-; CHECK-P9-BE-NEXT:    vsldoi v3, v2, v2, 13
-; CHECK-P9-BE-NEXT:    stxsibx vs35, 0, r5
+; CHECK-P9-BE-NEXT:    li r3, 3
 ; CHECK-P9-BE-NEXT:    vsldoi v3, v2, v2, 5
+; CHECK-P9-BE-NEXT:    stxsibx vs34, r5, r3
 ; CHECK-P9-BE-NEXT:    li r3, 1
 ; CHECK-P9-BE-NEXT:    stxsibx vs35, r5, r3
 ; CHECK-P9-BE-NEXT:    vsldoi v3, v2, v2, 2
 ; CHECK-P9-BE-NEXT:    li r3, 2
 ; CHECK-P9-BE-NEXT:    stxsibx vs35, r5, r3
-; CHECK-P9-BE-NEXT:    li r3, 3
 ; CHECK-P9-BE-NEXT:    vsldoi v3, v2, v2, 15
-; CHECK-P9-BE-NEXT:    stxsibx vs34, r5, r3
 ; CHECK-P9-BE-NEXT:    li r3, 4
 ; CHECK-P9-BE-NEXT:    stxsibx vs35, r5, r3
+; CHECK-P9-BE-NEXT:    vsldoi v3, v2, v2, 13
+; CHECK-P9-BE-NEXT:    stxsibx vs35, 0, r5
 ; CHECK-P9-BE-NEXT:    vsldoi v3, v2, v2, 9
 ; CHECK-P9-BE-NEXT:    li r3, 5
 ; CHECK-P9-BE-NEXT:    stxsibx vs35, r5, r3
 ; CHECK-P9-BE-NEXT:    vsldoi v3, v2, v2, 4
 ; CHECK-P9-BE-NEXT:    li r3, 6
 ; CHECK-P9-BE-NEXT:    stxsibx vs35, r5, r3
 ; CHECK-P9-BE-NEXT:    vsldoi v3, v2, v2, 6
 ; CHECK-P9-BE-NEXT:    li r3, 7

llvm/test/CodeGen/PowerPC/scheduling-mem-dependency.ll

 ; REQUIRES: asserts
 ; RUN: llc < %s -mtriple=powerpc64le-unknown-linux-gnu -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s
+; RUN: llc < %s -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr9 -verify-misched -debug-only=machine-scheduler -o - 2>&1 > /dev/null | FileCheck %s --check-prefix=CHECK-P9
 
 define i64 @store_disjoint_memory(i64* nocapture %P, i64 %v) {
 entry:
 ; CHECK: ********** MI Scheduling **********
 ; CHECK-LABEL: store_disjoint_memory:%bb.0
-; CHECK:SU(2):   STD renamable $x4, 24, renamable $x5 :: (store 8 into %ir.arrayidx)
+; CHECK:SU([[REG2:[0-9]+]]):   STD renamable $x{{[0-9]+}}, 24, renamable $x[[REG5:[0-9]+]]
 ; CHECK-NOT: Successors:
-; CHECK-NOT:    SU(3): Ord  Latency=0 Memory
-; CHECK:SU(3):   STD renamable $x4, 16, renamable $x5 :: (store 8 into %ir.arrayidx1)
+; CHECK-NOT:    SU([[REG3]]): Ord  Latency=0 Memory
+; CHECK:SU([[REG3:[0-9]+]]):   STD renamable $x{{[0-9]+}}, 16, renamable $x[[REG5]]
 ; CHECK: Predecessors:
-; CHECK-NOT:    SU(2): Ord  Latency=0 Memory
+; CHECK-NOT:    SU([[REG2]]): Ord  Latency=0 Memory
   %arrayidx = getelementptr inbounds i64, i64* %P, i64 3
   store i64 %v, i64* %arrayidx
   %arrayidx1 = getelementptr inbounds i64, i64* %P, i64 2
   store i64 %v, i64* %arrayidx1
   ret i64 %v
 }
+
+; LXSD is an instruction that can be modeled.
+@gd = external local_unnamed_addr global [500 x double], align 8
+@gf = external local_unnamed_addr global [500 x float], align 4
+
+define double @test_lxsd_no_barrier(double %a, double %b, double %c, double %d, double %e, double %f, double %g, double %h, double %i, double %j, double %k, double %l, double %m) {
+entry:
+  %0 = load double, double* getelementptr inbounds ([500 x double], [500 x double]* @gd, i64 0, i64 10), align 8
+  %1 = load double, double* getelementptr inbounds ([500 x double], [500 x double]* @gd, i64 0, i64 17), align 8
+  %2 = load double, double* getelementptr inbounds ([500 x double], [500 x double]* @gd, i64 0, i64 87), align 8
+  %3 = load double, double* getelementptr inbounds ([500 x double], [500 x double]* @gd, i64 0, i64 97), align 8
+  %4 = load double, double* getelementptr inbounds ([500 x double], [500 x double]* @gd, i64 0, i64 77), align 8
+  %add = fadd double %a, %b
+  %add1 = fadd double %add, %c
+  %add2 = fadd double %add1, %d
+  %add3 = fadd double %add2, %e
+  %add4 = fadd double %add3, %f
+  %add5 = fadd double %add4, %g
+  %add6 = fadd double %add5, %h
+  %add7 = fadd double %add6, %i
+  %add8 = fadd double %add7, %j
+  %add9 = fadd double %add8, %k
+  %add10 = fadd double %add9, %l
+  %add11 = fadd double %add10, %m
+  %add12 = fadd double %add11, %0
+  %add13 = fadd double %add12, %1
+  %add14 = fadd double %add13, %2
+  %add15 = fadd double %add14, %3
+  %add16 = fadd double %add15, %4
+  ret double %add16
+; CHECK-P9: ********** MI Scheduling **********
+; CHECK-P9-LABEL: test_lxsd_no_barrier:%bb.0 entry
+; CHECK-P9-NOT:Global memory object and new barrier chain: SU({{[0-9]+}}).
+; CHECK-P9:SU({{[0-9]+}}):   renamable $vf{{[0-9]+}} = LXSD 136
+; CHECK-P9:SU({{[0-9]+}}):   renamable $vf{{[0-9]+}} = LXSD 696
+; CHECK-P9:SU({{[0-9]+}}):   renamable $vf{{[0-9]+}} = LXSD 776
+; CHECK-P9:SU({{[0-9]+}}):   renamable $vf{{[0-9]+}} = LXSD 616
+}