[PATCH] D102073: [TargetLowering] Legalize "vscale x 1" types in more cases

Fri May 7 09:40:51 PDT 2021

frasercrmck added inline comments.

================
Comment at: llvm/test/CodeGen/RISCV/rvv/legalize-scalable-vectortype.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-v -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-v -verify-machineinstrs < %s | FileCheck %s
----------------
craig.topper wrote:
> frasercrmck wrote:
> > craig.topper wrote:
> > > These tests don't cover widening do they?
> > True, they don't. To be honest I was specifically targeting the promotion but the code looks like it should handle widening okay. I'm not sure we have an RVV case where we have to widen an illegal `<vscale x 1 x ty>` type to a legal one.
> I can't remember if the caller can handle widening scalable vectors properly. I fought with this code before when we tried to use types like <vscale x 6 x ty> for x6 segment load/stores.
> 
> Can the vectorize create these <vscale x 4 x i5> types that need promotion?
Yeah I wouldn't be surprised. Since this patch only affects `vscale x 1` types, any existing issues in widening scalable vectors are already out in the open. I think without proper tests, ot might be best to remove the claim of "allowing" widening from this patch. At least we can say it now treats `vscale x 1` more similarly to the others.

I haven't looked too deeply but I believe these `i5`s can appear during switch generation or switch case optimizations. I don't know about the loop vectorizer, but our WFV vectorizer will happily vectorize these.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102073