[PATCH] D110319: [SelectionDAG] Fixed the scalable vectors issue on WidenVecRes_OverflowOp&WidenVecRes_SELECT

[eric tang via Phabricator via llvm-commits]

tangxingxin1008 added a comment.

In D110319#3017678 <https://reviews.llvm.org/D110319#3017678>, @frasercrmck wrote:

> I know they're both small changes but I think it would be good to split this patch up into separate parts: overflow and select. They're conceptually distinct changes. Then we could have more targeted tests for each operation and each change.
>
> Also RISC-V would benefit from the `select` change, e.g., adding this sort of test to `test/CodeGen/RISCV/rvv/vselect-int-rv32.ll` and `vselect-int-rv64.ll`
>
>   define <vscale x 3 x i8> @vmerge_vv_nxv3i8(<vscale x 3 x i8> %va, <vscale x 3 x i8> %vb, <vscale x 3 x i1> %cond) {
>     %vc = select <vscale x 3 x i1> %cond, <vscale x 3 x i8> %va, <vscale x 3 x i8> %vb
>     ret <vscale x 3 x i8> %vc
>   }

thanks, I will be changed according to your advice.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp:854
 void DAGTypeLegalizer::SetWidenedVector(SDValue Op, SDValue Result) {
-  assert(Result.getValueType() ==
-         TLI.getTypeToTransformTo(*DAG.getContext(), Op.getValueType()) &&
+  assert(Result.getValueType().getVectorElementType() ==
+             TLI.getTypeToTransformTo(*DAG.getContext(), Op.getValueType())
----------------
frasercrmck wrote:
> This looks suspicious to me. What's it for?
In AArch sve, only [nxv2i1, nxv4i1, nxv8i1, nxv16i1] [nxv16i8, nxv8i16, nxv4i32, nxv2i64] are legal, so <nxv1i8, nxv1i1> would be widened to <nxv16i8, nxv16i1> by WidenVecRes_OverflowOp.
In below code:
  unsigned OtherNo = 1 - ResNo;
  EVT OtherVT = N->getValueType(OtherNo);
  if (getTypeAction(OtherVT) == TargetLowering::TypeWidenVector) {
    SetWidenedVector(SDValue(N, OtherNo), SDValue(WideNode, OtherNo));
  } 

getTypeAction(OtherVT) would convert <nxv1i1> to <nxv2i1>.

So the SDValue(N, OtherNo) value type is <nxv2i1>, but SDValue(WideNode, OtherNo) is <nxv16i1>. In this case, SetWidenedVector will be failed in 
  assert(Result.getValueType() ==
             TLI.getTypeToTransformTo(*DAG.getContext(), Op.getValueType()) &&
             "Invalid type for widened vector");
In my understand, I do this change like as SetSplitVector does. is that right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110319