[PATCH] D71391: [PowerPC] Modify the hasSideEffects of some VSX instructions from 1 to 0

[qshanz via Phabricator via llvm-commits]

steven.zhang added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrAltivec.td:1369
 // Vector Extract Unsigned Byte/Halfword/Word Left/Right-Indexed
+let hasSideEffects = 0 in {
 def VEXTUBLX : VX1_RT5_RA5_VB5<1549, "vextublx", []>;
----------------
ZhangKang wrote:
> Jim wrote:
> > steven.zhang wrote:
> > > Jim wrote:
> > > > hasSideEffects = 0 should be added class `VX1_RT5_RA5_VB5`.
> > > > Something like:
> > > > ```
> > > > let hasSideEffects = 0 in
> > > > class VX1_RT5_RA5_VB5<bits<11> xo, string opc, list<dag> pattern>
> > > > ...
> > > > ```
> > > I don't think it is good idea to clear this flag in the format class definition, except we have clear semantics that, there won't be any side effect for all the instructions with this format. But I don't see it, though, for now, it only have six instructions instance.  What do you think ?
> > For now, It seems that `VX1_RT5_RA5_VB5` is only for VEXTU* instructions. If any new instruction is added with this format, we can refactor the code for need.
> Yes, I will update the patch to set the `hasSideEffects` in `VX1_RT5_RA5_VB5 `.
Set the flag for the instruction format is NOT right. Please move it to the instruction definition and run the benchmark. If everything goes well, we can go with this patch.


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

https://reviews.llvm.org/D71391