[RISCV] vp.merge of nxv16i1 tries to be unrolled

[lukel97]

After #101641, this vp merge began to be emitted from the loop vectorizer with EVL tail folding from SPEC CPU 2017:

define <vscale x 16 x i1> @f(<vscale x 16 x i1> %m, <vscale x 16 x i1> %x, <vscale x 16 x i1> %y, i32 %evl) {
  %1 = tail call <vscale x 16 x i1> @llvm.vp.merge.nxv16i1(<vscale x 16 x i1> %m, <vscale x 16 x i1> %x, <vscale x 16 x i1> %y, i32 %evl)
  ret <vscale x 16 x i1> %1
}

It crashes with llc -mtriple=riscv64 -mattr=+v with LLVM ERROR: Invalid size request on a scalable vector. because it tries to unroll it when legalizing vector ops. The DAG after type legalization looks like:

Type-legalized selection DAG: %bb.0 'f:'
SelectionDAG has 15 nodes:
  t0: ch,glue = EntryToken
      t2: nxv16i1,ch = CopyFromReg t0, Register:nxv16i1 %0
      t4: nxv16i1,ch = CopyFromReg t0, Register:nxv16i1 %1
      t6: nxv16i1,ch = CopyFromReg t0, Register:nxv16i1 %2
        t8: i64,ch = CopyFromReg t0, Register:i64 %3
      t18: i64 = and t8, Constant:i64<4294967295>
    t12: nxv16i1 = vp_merge t2, t4, t6, t18
  t15: ch,glue = CopyToReg t0, Register:nxv16i1 $v0, t12
  t16: ch = RISCVISD::RET_GLUE t15, Register:nxv16i1 $v0, t15:1

[llvmbot]

@llvm/issue-subscribers-backend-risc-v

Author: Luke Lau (lukel97)

After #101641, this vp merge began to be emitted from the loop vectorizer with EVL tail folding from SPEC CPU 2017:

define &lt;vscale x 16 x i1&gt; @<!-- -->f(&lt;vscale x 16 x i1&gt; %m, &lt;vscale x 16 x i1&gt; %x, &lt;vscale x 16 x i1&gt; %y, i32 %evl) {
  %1 = tail call &lt;vscale x 16 x i1&gt; @<!-- -->llvm.vp.merge.nxv16i1(&lt;vscale x 16 x i1&gt; %m, &lt;vscale x 16 x i1&gt; %x, &lt;vscale x 16 x i1&gt; %y, i32 %evl)
  ret &lt;vscale x 16 x i1&gt; %1
}

It crashes with llc -mtriple=riscv64 -mattr=+v with LLVM ERROR: Invalid size request on a scalable vector. because it tries to unroll it when legalizing vector ops. The DAG after type legalization looks like:

Type-legalized selection DAG: %bb.0 'f:'
SelectionDAG has 15 nodes:
  t0: ch,glue = EntryToken
      t2: nxv16i1,ch = CopyFromReg t0, Register:nxv16i1 %0
      t4: nxv16i1,ch = CopyFromReg t0, Register:nxv16i1 %1
      t6: nxv16i1,ch = CopyFromReg t0, Register:nxv16i1 %2
        t8: i64,ch = CopyFromReg t0, Register:i64 %3
      t18: i64 = and t8, Constant:i64<4294967295>
    t12: nxv16i1 = vp_merge t2, t4, t6, t18
  t15: ch,glue = CopyToReg t0, Register:nxv16i1 $v0, t12
  t16: ch = RISCVISD::RET_GLUE t15, Register:nxv16i1 $v0, t15:1

[fhahn]

cc @Mel-Chen

[lukel97]

I think we should be able to lower this in the RISC-V backend, and I think it makes sense to transform selects to VP nodes during EVL tail folding since it allows us to get rid of a mask user.

But I think this goes to show that VP intrinsics aren't as well supported as regular, non-predicated instructions in the RISC-V backend.

Now that the VLOptimizer pass is enabled by default, do we want to reconsider emitting VP intrinsics during EVL tail folding if they're not necessary? I.e. not selects/loads/stores, but binary ops/other intrinsics.

My gut feeling is that we'll be missing out on a lot of combines and optimisations that just aren't implemented for VP intrinsics/nodes.

In that case we should probably emit regular instructions where possible in the loop vectorizer, then teach DAGCombiner and friends to handle VP nodes first. Then we could revisit emitting VP intrinsics once the codegen is a bit better.

(This is somewhat speculation: It would be good to evaluate this first! But to do that we need to fix the assertion failures 🥲)

cc @Mel-Chen @arcbbb @LiqinWeng @preames @npanchen @michaelmaitland

[topperc]

We probably have a fix for this in our downstream. We've been using VP intrinsics in our vectorizer for a while. I'll see if I can find it.

[fhahn]

How can we get such issues surfaced upstream sooner? It sounds like it might be good to put a hold on future EVL patches in LoopVectorize until all issues with the current state are fixed.

[preames]

How can we get such issues surfaced upstream sooner? It sounds like it might be good to put a hold on future EVL patches in LoopVectorize until all issues with the current state are fixed.

Well, at least some of the current EVL patches are addressing functional issues, so pausing that would seem slightly self defeating. However, I do agree that we need to get the EVL stuff into a correct working state before continuing to work on optimization quality. This does seem to be happening, but it needs to be more of a focus. @lukel97's recent test runs (and reporting) is a great help here, but we clearly need to be doing more of this.

[fhahn]

Well, at least some of the current EVL patches are addressing functional issues, so pausing that would seem slightly self defeating. However, I do agree that we need to get the EVL stuff into a correct working state before continuing to work on optimization quality. This does seem to be happening, but it needs to be more of a focus. @lukel97's recent test runs (and reporting) is a great help here, but we clearly need to be doing more of this.

Yep I didn't mean any patches addressing issues, only patches generating more vp.* intrinsics