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

Qing Shan Zhang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 28 19:35:07 PDT 2019


steven.zhang marked an inline comment as done.
steven.zhang added a comment.

In D69232#1724229 <https://reviews.llvm.org/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.



================
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
----------------
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.


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

https://reviews.llvm.org/D69232





More information about the llvm-commits mailing list