[CodeGen] Let computeKnownBits do something sensible for scalable vectors

[david-arm]

Summary

Until we have a real need for computing known bits for scalable
vectors I have simply changed the code to bail out for now and
pretend we know nothing. I've also fixed up some simple callers of
computeKnownBits too.

[efriedma]

I think we should be able to make computeKnownBits work to some extent, but I guess it's not high priority at the moment.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

2278

If we're going to bail out anyway, can we bail out explicitly, instead of waiting for computeKnownBits to bail out?

[david-arm]

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

2278

I thought about that, but then I was also worried about duplicating the behaviour of the called routines in the callers. If the behaviour of MaskedValueIsZero or computeKnownBits changes, i.e. how we return values and so on, then this becomes out of sync. If you don't think that's a problem I can bail out early as you suggested?

[david-arm]

Hi @efriedma, I don't suppose you've had any further thoughts about my reply to your comment? I was specifically worried that also bailing out early for all functions that call computeKnownBits means we're reliant on the behaviour of computeKnownBits not changing, i.e. in terms of it's returned values. If you don't think this is a concern, then I'm happy to do that.

[efriedma]

It's not clear how we actually want to represent DemandedElts for scalable vectors. I think we'd go through all the callers before changing anything in that respect, so explicitly bailing out seems better than intentionally synthesizing something which might be wrong anyway.

For MaskedValueIsZero specifically, maybe makes sense to change the implementation to return Mask.isSubsetOf(computeKnownBits(V, Depth).Zero);, instead of trying to synthesize DemandedElts.

[david-arm]

Hi @efriedma, thanks for the reply! Your comment about changing MaskedValueIsZero makes a lot of sense and improves code reuse. Hopefully the patch looks better now!

[efriedma]

The code looks right.

For both this and the SimplifyDemandedBits/VectorElts patches, I'm not sure what we want to do about testing. Currently, I assume some optimizations "work", and this breaks them. And probably we do some incorrect optimizations. It would be nice if we could have at least a little test coverage showing these changes have some effect.

[david-arm]

Hi @efriedma, I tried to find some pre-existing folds that we were doing before my patch, but couldn't find anything. The problem is that fundamentally many of these functions need to understand SPLAT_VECTORs in order to return something known. I tried adding SPLAT_VECTOR support to isConstOrConstSplat, since that allowed us to call MaskedValueIsZero a bit more. However, we then end up crashing in SimplifyDemandedBits. However, I do understand your concern so I have added a unit test that confirms we pretend we know nothing. This patch kills off the last remaining warnings in quite a few tests, so I have added checks for no warnings in those files.

[efriedma]

LGTM

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

bool SelectionDAG::SignBitIsZero(SDValue Op, unsigned Depth) const {
   return MaskedValueIsZero(Op, APInt::getSignMask(BitWidth), Depth);
 }
 
 /// MaskedValueIsZero - Return true if 'V & Mask' is known to be zero.  We use
 /// this predicate to simplify operations downstream.  Mask is known to be zero
 /// for bits that V cannot have.
 bool SelectionDAG::MaskedValueIsZero(SDValue V, const APInt &Mask,
                                      unsigned Depth) const {
-  EVT VT = V.getValueType();
-  APInt DemandedElts = VT.isVector()
-                           ? APInt::getAllOnesValue(VT.getVectorNumElements())
-                           : APInt(1, 1);
-  return MaskedValueIsZero(V, Mask, DemandedElts, Depth);
+  return Mask.isSubsetOf(computeKnownBits(V, Depth).Zero);
 }
 
 /// MaskedValueIsZero - Return true if 'V & Mask' is known to be zero in
 /// DemandedElts.  We use this predicate to simplify operations downstream.
 /// Mask is known to be zero for bits that V cannot have.
 bool SelectionDAG::MaskedValueIsZero(SDValue V, const APInt &Mask,
                                      const APInt &DemandedElts,
                                      unsigned Depth) const {
const APInt *SelectionDAG::getValidMaximumShiftAmountConstant(
   return MaxShAmt;
 }
 
 /// Determine which bits of Op are known to be either zero or one and return
 /// them in Known. For vectors, the known bits are those that are shared by
 /// every vector element.
 KnownBits SelectionDAG::computeKnownBits(SDValue Op, unsigned Depth) const {
   EVT VT = Op.getValueType();
+
+  // TOOD: Until we have a plan for how to represent demanded elements for
+  // scalable vectors, we can just bail out for now.
+  if (Op.getValueType().isScalableVector()) {
+    unsigned BitWidth = Op.getScalarValueSizeInBits();
+    return KnownBits(BitWidth);
+  }
+
   APInt DemandedElts = VT.isVector()
                            ? APInt::getAllOnesValue(VT.getVectorNumElements())
                            : APInt(1, 1);
   return computeKnownBits(Op, DemandedElts, Depth);
 }
 
 /// Determine which bits of Op are known to be either zero or one and return
 /// them in Known. The DemandedElts argument allows us to only collect the known
 /// bits that are shared by the requested vector elements.
 KnownBits SelectionDAG::computeKnownBits(SDValue Op, const APInt &DemandedElts,
                                          unsigned Depth) const {
   unsigned BitWidth = Op.getScalarValueSizeInBits();
 
   KnownBits Known(BitWidth);   // Don't know anything.
 
+  // TOOD: Until we have a plan for how to represent demanded elements for
+  // scalable vectors, we can just bail out for now.
+  if (Op.getValueType().isScalableVector())
+    return Known;
+
   if (auto *C = dyn_cast<ConstantSDNode>(Op)) {
     // We know all of the bits for a constant!
     Known.One = C->getAPIntValue();
     Known.Zero = ~Known.One;
     return Known;
   }
   if (auto *C = dyn_cast<ConstantFPSDNode>(Op)) {
     // We know all of the bits for a constant fp!

llvm/test/CodeGen/AArch64/sve-intrinsics-gather-prefetches-scalar-base-vector-indexes.ll

-; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve --asm-verbose=false < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve --asm-verbose=false < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
 
 ; PRFB <prfop>, <Pg>, [<Xn|SP>, <Zm>.S, <mod>]    -> 32-bit indexes
 define void @llvm_aarch64_sve_prfb_gather_uxtw_index_nx4vi32(<vscale x 4 x i1> %Pg, i8* %base, <vscale x 4 x i32> %indexes) nounwind {
 ; CHECK-LABEL: llvm_aarch64_sve_prfb_gather_uxtw_index_nx4vi32:
 ; CHECK-NEXT:  prfb  pldl1strm, p0, [x0, z0.s, uxtw]
 ; CHECK-NEXT:  ret
   call void @llvm.aarch64.sve.prfb.gather.uxtw.index.nx4vi32(<vscale x 4 x i1> %Pg, i8* %base, <vscale x 4 x i32> %indexes, i32 1)
   ret void

llvm/test/CodeGen/AArch64/sve-intrinsics-ld1ro.ll

-; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve,+f64mm -asm-verbose=0 < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve,+f64mm -asm-verbose=0 < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
 
 ;
 ; LD1ROB
 ;
 
 define <vscale x 16 x i8> @ld1rob_i8(<vscale x 16 x i1> %pred, i8* %addr) nounwind {
 ; CHECK-LABEL: ld1rob_i8:
 ; CHECK-NEXT:  ld1rob { z0.b }, p0/z, [x0]

llvm/test/CodeGen/AArch64/sve-intrinsics-scatter-stores-32bit-scaled-offsets.ll

-; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
 
 ;
 ; ST1H, ST1W, ST1D: base + 32-bit scaled offset, sign (sxtw) or zero
 ; (uxtw) extended to 64 bits.
 ;   e.g. st1h { z0.d }, p0, [x0, z1.d, uxtw #1]
 ;
 
 ; ST1H

llvm/test/CodeGen/AArch64/sve-intrinsics-scatter-stores-32bit-unscaled-offsets.ll

-; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
 
 ;
 ; ST1B, ST1W, ST1H, ST1D: base + 32-bit unscaled offset, sign (sxtw) or zero
 ; (uxtw) extended to 64 bits.
 ;   e.g. st1h { z0.d }, p0, [x0, z1.d, uxtw]
 ;
 
 ; ST1B

llvm/test/CodeGen/AArch64/sve-masked-ldst-nonext.ll

-; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve -asm-verbose=0 < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve -asm-verbose=0 < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
 
 ;
 ; Masked Loads
 ;
 
 define <vscale x 2 x i64> @masked_load_nxv2i64(<vscale x 2 x i64> *%a, <vscale x 2 x i1> %mask) nounwind {
 ; CHECK-LABEL: masked_load_nxv2i64:
 ; CHECK-NEXT: ld1d { z0.d }, p0/z, [x0]

llvm/test/CodeGen/AArch64/sve-masked-ldst-sext.ll

-; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
 
 ;
 ; Masked Loads
 ;
 
 define <vscale x 2 x i64> @masked_sload_nxv2i8(<vscale x 2 x i8> *%a, <vscale x 2 x i1> %mask) {
 ; CHECK-LABEL: masked_sload_nxv2i8:
 ; CHECK: ld1sb { [[IN:z[0-9]+]].d }, [[PG:p[0-9]+]]/z, [x0]

llvm/test/CodeGen/AArch64/sve-masked-ldst-trunc.ll

-; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve -asm-verbose=0 < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve -asm-verbose=0 < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
 
 ;
 ; Masked Stores
 ;
 
 define void @masked_trunc_store_nxv2i8(<vscale x 2 x i64> *%a, <vscale x 2 x i64> %val, <vscale x 2 x i8> *%b, <vscale x 2 x i1> %mask) nounwind {
 ; CHECK-LABEL: masked_trunc_store_nxv2i8:
 ; CHECK-NEXT: st1b { z0.d }, p0, [x1]

llvm/test/CodeGen/AArch64/sve-masked-ldst-zext.ll

-; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
 
 ;
 ; Masked Loads
 ;
 
 define <vscale x 2 x i64> @masked_zload_nxv2i8(<vscale x 2 x i8>* %src, <vscale x 2 x i1> %mask) {
 ; CHECK-LABEL: masked_zload_nxv2i8:
 ; CHECK-NOT: ld1sb

llvm/test/CodeGen/AArch64/sve-pred-contiguous-ldst-addressing-mode-reg-imm.ll

-; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve --asm-verbose=false < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve --asm-verbose=false < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
 
 ; Range checks: for all the instruction tested in this file, the
 ; immediate must be within the range [-8, 7] (4-bit immediate). Out of
 ; range values are tested only in one case (following). Valid values
 ; are tested all through the rest of the file.
 
 define void @imm_out_of_range(<vscale x 2 x i64> * %base, <vscale x 2 x i1> %mask) nounwind {
 ; CHECK-LABEL: imm_out_of_range:

llvm/test/CodeGen/AArch64/sve-pred-contiguous-ldst-addressing-mode-reg-reg.ll

-; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve --asm-verbose=false < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve --asm-verbose=false < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
 
 ; 2-lane contiguous load/stores
 
 define void @test_masked_ldst_sv2i8(i8 * %base, <vscale x 2 x i1> %mask, i64 %offset) nounwind {
 ; CHECK-LABEL: test_masked_ldst_sv2i8:
 ; CHECK-NEXT: ld1sb { z[[DATA:[0-9]+]].d }, p0/z, [x0, x1]
 ; CHECK-NEXT: st1b { z[[DATA]].d }, p0, [x0, x1]
 ; CHECK-NEXT: ret

llvm/test/CodeGen/AArch64/sve-setcc.ll

-; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; WARN-NOT: warning
 
 ; Ensure we use the inverted CC result of SVE compare instructions when branching.
 define void @sve_cmplt_setcc_inverted(<vscale x 8 x i16>* %out, <vscale x 8 x i16> %in, <vscale x 8 x i1> %pg) {
 ; CHECK-LABEL: @sve_cmplt_setcc_inverted
 ; CHECK: cmplt p1.h, p0/z, z0.h, #0
 ; CHECK-NEXT: ptest p0, p1.b
 ; CHECK-NEXT: b.ne
 entry:

llvm/unittests/CodeGen/AArch64SelectionDAGTest.cpp

TEST_F(AArch64SelectionDAGTest, computeKnownBits_ZERO_EXTEND_VECTOR_INREG) {
   auto OutVecVT = EVT::getVectorVT(Context, Int16VT, 2);
   auto InVec = DAG->getConstant(0, Loc, InVecVT);
   auto Op = DAG->getNode(ISD::ZERO_EXTEND_VECTOR_INREG, Loc, OutVecVT, InVec);
   auto DemandedElts = APInt(2, 3);
   KnownBits Known = DAG->computeKnownBits(Op, DemandedElts);
   EXPECT_TRUE(Known.isZero());
 }
 
+TEST_F(AArch64SelectionDAGTest, computeKnownBitsSVE_ZERO_EXTEND_VECTOR_INREG) {
+  if (!TM)
+    return;
+  SDLoc Loc;
+  auto Int8VT = EVT::getIntegerVT(Context, 8);
+  auto Int16VT = EVT::getIntegerVT(Context, 16);
+  auto InVecVT = EVT::getVectorVT(Context, Int8VT, 4, true);
+  auto OutVecVT = EVT::getVectorVT(Context, Int16VT, 2, true);
+  auto InVec = DAG->getConstant(0, Loc, InVecVT);
+  auto Op = DAG->getNode(ISD::ZERO_EXTEND_VECTOR_INREG, Loc, OutVecVT, InVec);
+  auto DemandedElts = APInt(2, 3);
+  KnownBits Known = DAG->computeKnownBits(Op, DemandedElts);
+
+  // We don't know anything for SVE at the moment.
+  EXPECT_EQ(Known.Zero, APInt(16, 0u));
+  EXPECT_EQ(Known.One, APInt(16, 0u));
+  EXPECT_FALSE(Known.isZero());
+}
+
 TEST_F(AArch64SelectionDAGTest, computeKnownBits_EXTRACT_SUBVECTOR) {
   if (!TM)
     return;
   SDLoc Loc;
   auto IntVT = EVT::getIntegerVT(Context, 8);
   auto VecVT = EVT::getVectorVT(Context, IntVT, 3);
   auto IdxVT = EVT::getIntegerVT(Context, 64);
   auto Vec = DAG->getConstant(0, Loc, VecVT);