[PATCH] D128286: [RISCV] Disable <vscale x 1 x *> types with Zve32x or Zve32f.

Thu Jun 23 09:09:42 PDT 2022

craig.topper added a comment.

In D128286#3605390 <https://reviews.llvm.org/D128286#3605390>, @reames wrote:

> Warning, I may be miss understanding the problem you're solving here, but...
>
> The case you mention appear to be specific to when VLEN=32 right?  If so, a cleaner way of phrasing the illegal cases would seem to be to compute the effective vector length after LMUL (i.e. VLEN/8 for mf8), and then disallow any case where the implied vector length is shorter than a single element of the element type.

The problem is still valid for VLEN>=64 ELEN=32. While you are correct that a SEW=8 LMUL=1/8 would fit in a register for that config, there is no requirement in the spec for hardware to support it. The spec says that that the smallest fractional LMUL is SEWMIN/ELEN. Which would be 8/32 or 1/4 in this config. The spec goes on to say "For a given supported fractional LMUL setting, implementations must support SEW settings between SEWMIN and LMUL * ELEN, inclusive." So if ELEN is 32, LMUL=1/4 is only required to support SEW=8 and LMUL=1/2 is only required to support SEW=8 or 16.

> A case to consider, what happens if VLEN=64?  Should we be disallowing e.g. mf2 e64?  (Seems like we should be right?)  If so, can we approach the problem the same?

mf2 e64 was already implicitly disabled. 1 x vscale x i64 is LMUL=1.

> The use of RVVBitsPerBlock feels like a red-herring here.  In particular, it's not clear to me why the value remains 64 on a VLEN=32 configuration.

You are correct RVVBitsPerBlock should change with Zve32 in order to support VLEN=32. Unfortunately, it would change the type mapping from vscale to LMUL and require substantial changes to the tablegen patterns. Naively I think it would roughly double the size of the isel table. Since we use MVT types to pick instructions register classes, we would need 2 sets of patterns.

If we did change RVVBitsPerBlock for Zve32 the problem I'm trying to fix here would go away.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128286