[PATCH] D92789: [PPC] Check for PPC64 when emitting 64bit specific VSX nodes when pattern matching built vectors

[Xiangling Liao via Phabricator via llvm-commits]

Xiangling_L added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9389
 
     // BUILD_VECTOR nodes that are not constant splats of up to 32-bits can be
     // lowered to VSX instructions under certain conditions.
----------------
minor: Is that better to adjust the comment accordingly like:
`Under 64-bit mode,  BUILD_VECTOR nodes that ...`


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:2412
 // [HasVSX]
 // [HasVSX, IsBigEndian]
 // [HasVSX, IsLittleEndian]
----------------
I am wondering why do we leave alone `[HasVSX, IsBigEndian]` and `[HasVSX, HasOnlySwappingMemOps, IsBigEndian]` and `HasVSX, HasP8Vector, NoP9Vector, IsBigEndian]`?


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:2427
 // [HasVSX, HasDirectMove, NoP9Altivec, IsLittleEndian]
-// [HasVSX, HasDirectMove, NoP9Vector, IsBigEndian]
+// [HasVSX, HasDirectMove, NoP9Vector, IsBigEndian, IsPPC64]
 // [HasVSX, HasDirectMove, NoP9Vector, IsLittleEndian]
----------------
minor: This line should follow #2425?


================
Comment at: llvm/test/CodeGen/PowerPC/ppc-32bit-build-vector.ll:3
+; RUN: llc -verify-machineinstrs -mtriple=powerpc \
+; RUN:     -mcpu=pwr8 -mattr=+altivec -mattr=+vsx < %s | FileCheck %s --check-prefix=32BIT
+
----------------
It seems `-mcpu=pwr8` already implies HasAltivec and HasVSX enabled, so `-mattr=+altivec` and  `-mattr=+vsx` are not necessary.


================
Comment at: llvm/test/CodeGen/PowerPC/ppc-32bit-build-vector.ll:67
+    %0 = insertelement <16 x i32> <i32 undef, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0>, i32 %newelement, i32 0
+    %load = load <16 x i8>, <16 x i8>* undef, align 1
+    %1 = zext <16 x i8> %load to <16 x i32>
----------------
Suggestion: The testcase can be further simplified into the following by removing tedious `zext` instruction and shortening the numbers of vector.
```
define dso_local fastcc void @BuildVectorICE() unnamed_addr #0 {
  entry:
    br label %while.body
    while.body:                                       ; preds = %while.body, %entry
    %newelement = phi i32 [ 0, %entry ], [ %5, %while.body ]
    %0 = insertelement <4 x i32> <i32 undef, i32 0, i32 0, i32 0>, i32 %newelement, i32 0
    %1 = load <4 x i32>, <4 x i32>* undef, align 1
    %2 = add <4 x i32> %1, %0
    %3 = shufflevector <4 x i32> %2, <4 x i32> undef, <4 x i32> <i32 1, i32 undef, i32 undef, i32 undef>
    %4 = add <4 x i32> %2, %3
    %5 = extractelement <4 x i32> %4, i32 0
    br label %while.body
}

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92789