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

[Jinsong Ji via Phabricator via llvm-commits]

jsji added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/mi-scheduling.ll:36
+; CHECK-LABEL: sideeffects:%bb.0 entry 
+; CHECK-NOT:Global memory object and new barrier chain: SU({{[0-9]+}}).
+; CHECK:SU({{[0-9]+}}):   renamable $vf{{[0-9]+}} = LXSD 136
----------------
steven.zhang wrote:
> jsji wrote:
> > 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?
> 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.
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?



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

https://reviews.llvm.org/D69232