[PATCH] D34077: DAGCombine: Combine BUILD_VECTOR to TRUNCATE

[Zvi Rackover via Phabricator via llvm-commits]

zvi added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14262
+    return SDValue();
+
+  // Compute the stride from the first index.
----------------
RKSimon wrote:
> Would it be useful to support BUILD_VECTORs containing UNDEF elements?
I put a TODO in line 14245 above as a reminder. I thought to start with a conservative approach in this patch and possibly add support for undefs later. The concern is that by replacing with a 'truncate' node  we will be losing the 'undef' element information.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14272
+      (VT.getScalarSizeInBits() * Stride > 64))
+    return SDValue();
+
----------------
RKSimon wrote:
> Check for ISD::TRUNCATE legality, this might even help ARM?
Thanks for suggesting that. Will give it a try.


================
Comment at: test/CodeGen/ARM/vext.ll:205
 ; CHECK-NEXT:    vzip.16 d17, d16
-; CHECK-NEXT:    vmov  r0, r1, d17
+; CHECK-NEXT:    vmov r0, r1, d17
 ; CHECK-NEXT:    mov pc, lr
----------------
RKSimon wrote:
> whitespace only change - if this is actually relevant commit it separately now as a NFC, else discard it.
ok


================
Comment at: test/CodeGen/X86/shuffle-vs-trunc-256.ll:2
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx2 | FileCheck %s --check-prefix=AVX --check-prefix=AVX2
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx512f | FileCheck %s --check-prefix=AVX512 --check-prefix=AVX512F
----------------
RKSimon wrote:
> Ideally it'd be good to see the codegen for AVX1 targets as well
ok


https://reviews.llvm.org/D34077