[PATCH] D33137: [DAGCombiner] use narrow vector ops to eliminate concat/extract (PR32790)

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
Herald added subscribers: mcrosier, rengolin, aemerson.

This patch started off a lot smaller, but then I discovered that bitcasts seem to always complicate the relatively simple pattern of:

  // extract (binop (concat X1, X2), (concat Y1, Y2)), N --> binop XN, YN

...and that made the code grow.

Hopefully, I've added enough comments to avoid too much confusion. If there's any way to simplify this, I'd be happy to hear it.

The TODO about extending to more than bitwise logic is there because we really will regress several x86 tests including madd, psad, and even a plain integer-multiply-by-2 or shift-left-by-1. I don't think there's anything fundamentally wrong with this patch that would cause those regressions; those folds are just missing or brittle.

If we extend to more binops, I found that this patch will fire on at least one non-x86 regression test. There's an ARM NEON test in test/CodeGen/ARM/coalesce-subregs.ll with a pattern like:

              t5: v2f32 = vector_shuffle<0,3> t2, t4
            t6: v1i64 = bitcast t5
            t8: v1i64 = BUILD_VECTOR Constant:i64<0>
          t9: v2i64 = concat_vectors t6, t8
        t10: v4f32 = bitcast t9
      t12: v4f32 = fmul t11, t10
    t13: v2i64 = bitcast t12
  t16: v1i64 = extract_subvector t13, Constant:i32<0>

There was no functional change in the codegen from this transform from what I could see though.

For the x86 test changes:

1. PR32790() is the closest call. We don't reduce the AVX1 instruction count in that case, but we improve throughput. Also, on a core like Jaguar that double-pumps 256-bit ops, there's an unseen win because two 128-bit ops have the same cost as the wider 256-bit op. SSE/AVX2/AXV512 are not affected which is expected because only AVX1 has the extract/concat ops to match the pattern.
2. do_not_use_256bit_op() is the best case. Everyone wins by avoiding the concat/extract. Related bug for IR filed as: https://bugs.llvm.org/show_bug.cgi?id=33026
3. The SSE diffs in vector-trunc-math.ll are just scheduling/RA, so nothing real AFAICT.
4. The AVX1 diffs in vector-tzcnt-256.ll are all the same pattern: we reduced the instruction count by one in each case by eliminating two insert/extract while adding one narrower logic op.


https://reviews.llvm.org/D33137

Files:
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  test/CodeGen/X86/vector-narrow-binop.ll
  test/CodeGen/X86/vector-trunc-math.ll
  test/CodeGen/X86/vector-tzcnt-256.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33137.98789.patch
Type: text/x-patch
Size: 27491 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170512/202e6eff/attachment.bin>