[PATCH] D119469: [AArch64] Turn truncating buildvectors into truncates

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:9269
+// concats and truncate nodes.
+static SDValue ReconstructTruncateFromBuildVector(SDValue V, SelectionDAG &DAG) {
+  if (V.getValueType() != MVT::v16i8)
----------------
david-arm wrote:
> dmgreen wrote:
> > david-arm wrote:
> > > Hi @dmgreen, this feels a bit *too* specific to a particular optimisation. For example, there is a test in CodeGen/AArch64/neon-extracttruncate.ll called `@extract_2_v4i32` that could also benefit from something like this. Also, some of the code in here looks very similar to what ReconstructShuffle attempts to do, which also looks at BUILD_VECTORS that are constructed from extractelement operations. Would it be better to simply extend ReconstructShuffle to cover this case?
> > ReconstructShuffle is for reconstructing buildvectors into Legal Shuffles, and if you look at the code only handles 2 sources. This method is for reconstructing a truncates, not shuffles, and needs to handle 4 sources. As this isn't a legal shuffle, it doesn't really fit there.
> > The tests in neon-extracttruncate.ll I just added as a quick test. They are not expected to come up in practice. https://godbolt.org/z/eEser71Gq. It is the lowering of large fptosi.sat that we are targeting.
> OK, but is it worth making this more generic then than it currently is? i.e. dealing with more cases than just this one very specific issue? @extract_2_v4i32 looks like it could benefit too.
Hmm. Like I showed in the godbolt link above extract_2_v4i32 will be converted to shuffles prior to ISel. It will go though normal shuffle codepaths, not being converted to a BUILD_VECTOR anywhere during ISel. This method cant really help with that, and won't expect to catch any cases which have been optimized from llvm into a shuffle already.

Do you have cases where you think a more general version of this function would come up?


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

https://reviews.llvm.org/D119469