[AArch64][SVEIntrinsicOpts] Fold sve_convert_from_svbool(zero) to zero

[joechrisellis]

Summary

Co-authored-by: Paul Walker <[e-mail]>

[dmgreen]

Why is this kind of thing not done in instcombine? Or even constant folding if that is what this is really doing.

[joechrisellis]

Hi @dmgreen! This is SVE-specific, and SVEIntrinsicOpts.cpp is where such transformations are typically placed (at least for now). I did a quick grep and it seems SVE intrinsics don't currently have much of a presence in generic passes like instcombine/constant folding, perhaps because some SVE optimisations are more complex than others and I guess it makes sense to keep them all in the same place.

X86 also has llvm/lib/Target/X86/X86InstCombineIntrinsic.cpp which houses instcombine-like optimisations for X86. 🙂

[dmgreen]

In D100463#2690659, @joechrisellis wrote:

Hi @dmgreen! This is SVE-specific, and SVEIntrinsicOpts.cpp is where such transformations are typically placed (at least for now). I did a quick grep and it seems SVE intrinsics don't currently have much of a presence in generic passes like instcombine/constant folding, perhaps because some SVE optimisations are more complex than others and I guess it makes sense to keep them all in the same place.

X86 also has llvm/lib/Target/X86/X86InstCombineIntrinsic.cpp which houses instcombine-like optimisations for X86. 🙂

Hmm. "Because we've been doing it wrong" isn't a great reason to _keep_ doing it wrong :-)

This is very similar to https://reviews.llvm.org/rGbecaa6803ab532d15506829f0551a5fa49c39d7e, but special cased for zeroinit vectors. I do see this code at the start of that function, but it seems like it could still be made to work, and presumably in the long run some of the other constant folding there might be useful (like masked loads with zero masks).

// Do not iterate on scalable vector. The number of elements is unknown at
// compile-time.
if (isa<ScalableVectorType>(VTy))
  return nullptr;

More generally, any backend can use instCombineIntrinsic to add intrinsic folds to instcombine. I can appreciate that not everything that SVECombines is doing will fit well there, but the ones that do should be put there if possible. The steady state combining it does will be worthwhile, as well as it being run multiple times through the pipeline. Plus, you know, not re-inventing the wheel.

[joechrisellis]

@dmgreen -- I hear you, makes sense to me. There might be some functionality already in SVEIntrinsicOpts.cpp that we can pull out, but I guess that can wait for later. In any case I'll move the functionality here to the other passes to reflect your suggestions.

Appreciate your comments! 😄

[joechrisellis]

Address review comments.

• @dmgreen: move transformation to constant folding.

[dmgreen]

Nice one, thanks. This Looks good to me.

llvm/lib/Analysis/ConstantFolding.cpp

3003

Doesn't need a break after the return (unless it's quieting a warning, but I think most compilers treat this sensibly).

llvm/test/Transforms/InstSimplify/ConstProp/AArch64/aarch64-sve-convert-from-svbool.ll

2

If this folder is new it may need (or at least be best) to have a lit.local.cfg to only run it when AArch64 is a registered target.

[joechrisellis]

Address review comments.

• @dmgreen:
  • Add lit.local.cfg file to AArch64 directory.
  • Fix minor editing error.

llvm/lib/Analysis/ConstantFolding.cpp

 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/IntrinsicsAArch64.h"
 #include "llvm/IR/IntrinsicsAMDGPU.h"
 #include "llvm/IR/IntrinsicsARM.h"
 #include "llvm/IR/IntrinsicsWebAssembly.h"
 #include "llvm/IR/IntrinsicsX86.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Support/Casting.h"
bool llvm::canConstantFoldCallTo(const CallBase *Call, const Function *F) {
   case Intrinsic::vector_reduce_smax:
   case Intrinsic::vector_reduce_umin:
   case Intrinsic::vector_reduce_umax:
   // Target intrinsics
   case Intrinsic::arm_mve_vctp8:
   case Intrinsic::arm_mve_vctp16:
   case Intrinsic::arm_mve_vctp32:
   case Intrinsic::arm_mve_vctp64:
+  case Intrinsic::aarch64_sve_convert_from_svbool:
   // WebAssembly float semantics are always known
   case Intrinsic::wasm_trunc_signed:
   case Intrinsic::wasm_trunc_unsigned:
   case Intrinsic::wasm_trunc_saturate_signed:
   case Intrinsic::wasm_trunc_saturate_unsigned:
     return true;
 
   // Floating point operations cannot be folded in strictfp functions in
if (Operands.size() == 2)
     return ConstantFoldScalarCall2(Name, IntrinsicID, Ty, Operands, TLI, Call);
 
   if (Operands.size() == 3)
     return ConstantFoldScalarCall3(Name, IntrinsicID, Ty, Operands, TLI, Call);
 
   return nullptr;
 }
 
-static Constant *ConstantFoldVectorCall(StringRef Name,
-                                        Intrinsic::ID IntrinsicID,
-                                        VectorType *VTy,
-                                        ArrayRef<Constant *> Operands,
-                                        const DataLayout &DL,
-                                        const TargetLibraryInfo *TLI,
-                                        const CallBase *Call) {
-  // Do not iterate on scalable vector. The number of elements is unknown at
-  // compile-time.
-  if (isa<ScalableVectorType>(VTy))
-    return nullptr;
-
-  auto *FVTy = cast<FixedVectorType>(VTy);
-
+static Constant *ConstantFoldFixedVectorCall(
+    StringRef Name, Intrinsic::ID IntrinsicID, FixedVectorType *FVTy,
+    ArrayRef<Constant *> Operands, const DataLayout &DL,
+    const TargetLibraryInfo *TLI, const CallBase *Call) {
   SmallVector<Constant *, 4> Result(FVTy->getNumElements());
   SmallVector<Constant *, 4> Lane(Operands.size());
   Type *Ty = FVTy->getElementType();
 
   switch (IntrinsicID) {
   case Intrinsic::masked_load: {
     auto *SrcPtr = Operands[0];
     auto *Mask = Operands[2];
for (unsigned I = 0, E = FVTy->getNumElements(); I != E; ++I) {
     if (!Folded)
       return nullptr;
     Result[I] = Folded;
   }
 
   return ConstantVector::get(Result);
 }
 
+static Constant *ConstantFoldScalableVectorCall(
+    StringRef Name, Intrinsic::ID IntrinsicID, ScalableVectorType *SVTy,
+    ArrayRef<Constant *> Operands, const DataLayout &DL,
+    const TargetLibraryInfo *TLI, const CallBase *Call) {
+  switch (IntrinsicID) {
+  case Intrinsic::aarch64_sve_convert_from_svbool: {
+    auto *Src = dyn_cast<Constant>(Operands[0]);
+    if (!Src || !Src->isNullValue())
+      break;
+
+    return ConstantInt::getFalse(SVTy);
+  }
+  default:
+    break;
+  }
+  return nullptr;
+}
+
 } // end anonymous namespace
 
 Constant *llvm::ConstantFoldCall(const CallBase *Call, Function *F,
                                  ArrayRef<Constant *> Operands,
                                  const TargetLibraryInfo *TLI) {
   if (Call->isNoBuiltin())
     return nullptr;
   if (!F->hasName())
     return nullptr;
   StringRef Name = F->getName();
 
   Type *Ty = F->getReturnType();
 
-  if (auto *VTy = dyn_cast<VectorType>(Ty))
-    return ConstantFoldVectorCall(Name, F->getIntrinsicID(), VTy, Operands,
-                                  F->getParent()->getDataLayout(), TLI, Call);
+  if (auto *FVTy = dyn_cast<FixedVectorType>(Ty))
+    return ConstantFoldFixedVectorCall(
+        Name, F->getIntrinsicID(), FVTy, Operands,
+        F->getParent()->getDataLayout(), TLI, Call);
+
+  if (auto *SVTy = dyn_cast<ScalableVectorType>(Ty))
+    return ConstantFoldScalableVectorCall(
+        Name, F->getIntrinsicID(), SVTy, Operands,
 
   return ConstantFoldScalarCall(Name, F->getIntrinsicID(), Ty, Operands, TLI,
                                 Call);
 }
 
 bool llvm::isMathLibCallNoop(const CallBase *Call,
                              const TargetLibraryInfo *TLI) {
   // FIXME: Refactor this code; this duplicates logic in LibCallsShrinkWrap

llvm/test/Transforms/InstSimplify/ConstProp/AArch64/aarch64-sve-convert-from-svbool.ll

+; RUN: opt -instsimplify -S -o - < %s | FileCheck %s
+
+define <vscale x 2 x i1> @reinterpret_zero() {
+; CHECK-LABEL:  @reinterpret_zero(
+; CHECK: ret <vscale x 2 x i1> zeroinitializer
+  %pg = tail call <vscale x 2 x i1> @llvm.aarch64.sve.convert.from.svbool.nxv2i1(<vscale x 16 x i1> zeroinitializer)
+  ret <vscale x 2 x i1> %pg
+}
+
+declare <vscale x 2 x i1> @llvm.aarch64.sve.convert.from.svbool.nxv2i1(<vscale x 16 x i1>)

llvm/test/Transforms/InstSimplify/ConstProp/AArch64/lit.local.cfg

+if not 'AArch64' in config.root.targets:
+    config.unsupported = True