[PATCH] D10555: [X86] Replace avx2.pbroadcast intrinsics with native IR.

[Simon Pilgrim via llvm-commits]

RKSimon added a comment.

In http://reviews.llvm.org/D10555#225988, @chandlerc wrote:

> FWIW, I really like this patch. Is there anything we can do to make this work?


It appears we have a few things that need to be decided before going any further:

1 - When is it permitable to replace a (sub)target-specific intrinsic with a non-specific implementation in the headers (e.g. using __builtin_shufflevector for these broadcasts)?

As long as the expected instruction remains in debug code I'm keen for this to be encouraged - we can add suitable tests, remove those builtin intrinsics to AutoUpgrade.cpp until 4.0 and get much cleaner headers.

2 - When is it permitable to replace a (sub)target-specific intrinsic in IR/DAG creation, and should that occur in InstCombine or in the target ISel code someplace?

I'd vote for InstCombine as we already appear to have a critical mass of intrinsics here.

3 - What are we going to do to fix the issue introduced by the header refactor removing the target guards, causing a tricky to decipher 'Cannot select: intrinsic %llvm.x86.vcvtps2ph.128' style backend error for intrinsics that are implemented as macros?

A quick+nasty solution would be to add header guards at least around each of those macros.


http://reviews.llvm.org/D10555