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

[Zhang Kang via Phabricator via llvm-commits]

ZhangKang marked an inline comment as done.
ZhangKang added a comment.

I have done the performance test for this patch, there is no degression.



================
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:
> steven.zhang wrote:
> > 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.
> Have updated the patch, I am testing the performance now.
I have done the performance test for this patch, there is no degression.


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

https://reviews.llvm.org/D71391