[SelectionDAG] Support scalable-vector splats in more cases

[frasercrmck]

Summary

This patch adds support for scalable-vector splats in DAGCombiner's
isConstantOrConstantVector and ISD::matchUnaryPredicate functions,
which enable the SelectionDAG div/rem-by-constant optimizations for
scalable vector types.

It also fixes up one case where the UDIV optimization was generating a
SETCC without first consulting the target for its preferred SETCC result
type.

[frasercrmck]

@c-rhodes or @david-arm I've updated the AArch64 tests naively but I thought I'd tag you in case you don't intend to be shifting out all bits.

[frasercrmck]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

5034

In general, is isFixedLengthVector || isScalableVector a suitable replacement for isVector, or should we be more explicit/future-proof about the possibility of a third case?

[craig.topper]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

5038

Is there only one element in Shifts and Factors? If I'm reading matchUnaryPredicate right it will only see the single operand of the SPLAT_VECTOR so only call BuildSDIVPattern once?

5329

I'm a bit surprised this didn't affect AVX512 on X86.

llvm/test/CodeGen/AArch64/sve-int-arith-imm.ll

598

Is this test trying to test an oversized shift amount or just a bad choice of constant? I'm leaning towards bad constant choice the lsl tests don't have this issue?

[frasercrmck]

rebase on main

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

5038

Yes, that's how it's working. Is that strong enough a thing to rely upon, do you think?

5329

Should we double-check that AVX512 has tested this code path?

llvm/test/CodeGen/AArch64/sve-int-arith-imm.ll

598

Yeah I'd imagine the latter too. I tried to tag some of the AArch64 team to see if they could help resolve this issue

[david-arm]

llvm/test/CodeGen/AArch64/sve-int-arith-imm.ll

598

Oh yeah, sorry about this. We did discuss it and the problem is with our tests I think. We've created tests that are shifting by more than the element width. I think this change is fine and we'll create some better tests after your patch lands.

[craig.topper]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

5038

Can we put an assert on the size and everwhere else we access element 0 to make it clear we aren't dropping anything?

[frasercrmck]

• add in assertions

[frasercrmck]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

5038

Yep, done

[craig.topper]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

5329

Looks like both operands to getSetCC are both constant vectors so this gets constant folded on non-scalable vectors. Then the getSelect will constant fold based on the condition being a constant. So maybe that's why AVX512 doesn't care.

[craig.topper]

LGTM

[huihuiz]

This is crashing SVE target lowering with "LLVM ERROR: Cannot select: t23: nxv4i32 = mulhu t9, t34"
Let me know if there is a way of fixing it?

Take below t.ll
Run: llc -O2 -mtriple=aarch64-linux-gnu -mattr=+sve < t.ll

target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
target triple = "aarch64-unknown-linux-gnu"

define void @test(<vscale x 4 x i32>* %in, <vscale x 4 x i32>* %out, <vscale x 4 x i1> %p) {
entry:
  %a = load <vscale x 4 x i32>, <vscale x 4 x i32>* %in, align 16
  %div = udiv <vscale x 4 x i32> %a, shufflevector (<vscale x 4 x i32> insertelement (<vscale x 4 x i32> undef, i32 3, i32 0), <vscale x 4 x i32> undef, <vscale x 4 x i32> zeroinitializer)
  call void @llvm.masked.store.nxv4i32.p0nxv4i32(<vscale x 4 x i32> %div, <vscale x 4 x i32>* %out, i32 4, <vscale x 4 x i1> %p)
  unreachable
}

declare void @llvm.masked.store.nxv4i32.p0nxv4i32(<vscale x 4 x i32>, <vscale x 4 x i32>*, i32 immarg, <vscale x 4 x i1>)

[frasercrmck]

In D94501#2566853, @huihuiz wrote:

This is crashing SVE target lowering with "LLVM ERROR: Cannot select: t23: nxv4i32 = mulhu t9, t34"
Let me know if there is a way of fixing it?

Hi @huihuiz, thanks for raising this issue with me. You've probably seen D96849 by now.

Since I'm not familiar with AArch64 I thought the quickest and safest way to fix the issue would be to prevent these optimizations from taking place. The AArch64 team can work out if there's a way of supporting these nodes.

[huihuiz]

Thanks @frasercrmck for helping! Appreciate it!