[PATCH] D125601: [DAGCombiner][AArch64] Reorder the bitcast of scalable vector

[Allen zhong via Phabricator via llvm-commits]

Allen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14937
+  if (InVT.isScalableVector() && InVT.isFloatingPoint() &&
+      DCI.isBeforeLegalize() && !OutVT.isScalableVector()) {
+    // Bitcast the input
----------------
Allen wrote:
> david-arm wrote:
> > From what I can see all the input and output types you're hoping to deal with are legal, right? For example, <vscale x 2 x float> is legal, but <vscale x 2 x i32> is illegal. So it looks like you're trying to take advantage of legalisation behaviour when doing something like this:
> > 
> >   %out = <2 x i32> extract_subvector <vscale x 2 x i32> %in, i32 0
> > 
> > which will probably turn into
> > 
> >   %out1 = <2 x i64> extract_subvector <vscale x 2 x i64> %in, i32 0
> >   %out2 = truncate <2 x i64> %out1 to <2 x i32>
> > 
> > The first operation will then become a nop.
> > 
> > One problem with this approach is that it looks like you're assuming the index is always 0. What happens when extracting a subvector from index 2, etc? I'm worried the generated code might then look even worse. It just feels like we might want to restrict the allowed cases a bit more here to just those examples where we know there will be an improvement.
> > 
> sure, I hope to deal with all the input and output types  are legal.  would you please show me which API can be used, here ? thanks.
hi @david-arm , do you mean to list all the type pair, where we know there will be an improvement?


================
Comment at: llvm/test/CodeGen/AArch64/extract-insert-element-sve.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=aarch64 -mattr=+sve -asm-verbose=1 < %s | FileCheck %s
----------------
david-arm wrote:
> We actually already have similar tests in sve-fixed-length-reshuffle.ll - could you move this test into that file please?
done, thanks.


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

https://reviews.llvm.org/D125601