[PATCH] D70436: [PowerPC] Add missing legalization for vector BSWAP

[qshanz via Phabricator via llvm-commits]

steven.zhang added a comment.
Herald added a subscriber: wuzish.





================
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:2950
   // Vector Byte-Reverse H/W/D/Q Word
   def XXBRH : XX2_XT6_XO5_XB6<60,  7, 475, "xxbrh", vsrc, []>;
+  def XXBRW : XX2_XT6_XO5_XB6<60, 15, 475, "xxbrw", vsrc,
----------------
It is surprising to me that, vsrc didn't have v16i8 and v1i128. However, it has nothing to do with this patch.  It would be great if someone could educate me on this :)


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:2962
+            (v8i16 (COPY_TO_REGCLASS (XXBRH (COPY_TO_REGCLASS $A, VSRC)), VRRC))>;
   def : Pat<(v4i32 (PPCxxreverse v4i32 :$A)),
             (v4i32 (XXBRW $A))>;
----------------
The PPCxxreverse has completely the same semantics of bswap. I will commit a follow up patch to clean up it, as it might stop us to leverage the combine rules for bswap that implemented in DAGCombiner.


================
Comment at: llvm/test/CodeGen/PowerPC/vec-bswap.ll:112
+; Function Attrs: argmemonly nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+
----------------
It is never used in the IR. Please remove it together with the end pair.


================
Comment at: llvm/test/CodeGen/PowerPC/vec-bswap.ll:123
+
+attributes #0 = { nounwind }
----------------
The attribute is also not needed.


================
Comment at: llvm/test/Transforms/LoopVectorize/PowerPC/vectorize-bswap.ll:4
+; RUN:   -force-target-max-vector-interleave=1 -mcpu=pwr9 < %s | FileCheck %s
+define dso_local void @test(i32* %Arr, i32 signext %Len) {
+; CHECK-LABEL: @test(
----------------
The dso_local here is not need i think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70436