[PATCH] D133700: [PowerPC] Exploit xxperm, check for dead vectors and substitute vperm with xxperm

[Maryam Moghadas via Phabricator via llvm-commits]

maryammo added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10294
+  LLVM_DEBUG(dbgs() << "With the following permute control vector:\n");
+  LLVM_DEBUG(VPermMask.dump());
+
----------------
stefanp wrote:
> Oh, I see. The second half of the debug comment is down here.
> It may be a good idea to move the two parts of the comment to the same place down here. 
> 
> Also, the initial part of the comment :
> ```
> Emitting a VPERM for the following shuffle:
> ```
> May not be true as this may now be an `XXPERM`.
The code staring in line 10163 for LLVB_DEBUG uses 


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10294
+  LLVM_DEBUG(dbgs() << "With the following permute control vector:\n");
+  LLVM_DEBUG(VPermMask.dump());
+
----------------
maryammo wrote:
> stefanp wrote:
> > Oh, I see. The second half of the debug comment is down here.
> > It may be a good idea to move the two parts of the comment to the same place down here. 
> > 
> > Also, the initial part of the comment :
> > ```
> > Emitting a VPERM for the following shuffle:
> > ```
> > May not be true as this may now be an `XXPERM`.
> The code staring in line 10163 for LLVB_DEBUG uses 
The LLVM_DEBUG code block starting at line 10163 uses SVOp which we dont pass it to LowerVPERM function as it is not needed here, that is why we have it there, I plan to delete LLVM_DEBUG from here and keep it there. Please lemme know if you have a concern.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:91
+  SDTCisVT<0, v2f64>, SDTCisVT<1, v2f64>,
+  SDTCisVT<2, v2f64>]>;
 //--------------------------- Custom PPC nodes -------------------------------//
----------------
stefanp wrote:
> Is it possible to add a type constraint for the last operand here?  
> ```
> SDTCisVT<3, v4i32>
> ```
> Or is that going to cause issues elsewhere?
The last one is  SDTCisVT<2, v2f64> that has a different type constraint, are you suggesting to change it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133700