[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 ↗

(On Diff #338044)

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

1 ↗

(On Diff #338044)

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/Target/AArch64/SVEIntrinsicOpts.cpp

 bool SVEIntrinsicOpts::optimizeConvertFromSVBool(IntrinsicInst *I) {
   assert(I->getIntrinsicID() == Intrinsic::aarch64_sve_convert_from_svbool &&
          "Unexpected opcode");
 
   // If the reinterpret instruction operand is a PHI Node
   if (isa<PHINode>(I->getArgOperand(0)))
     return processPhiNode(I);
 
+  if (auto *C = dyn_cast<Constant>(I->getArgOperand(0))) {
+    if (!C->isNullValue())
+      return false;
+
+    I->replaceAllUsesWith(ConstantInt::getFalse(I->getType()));
+    I->eraseFromParent();
+    return true;
+  }
+
   SmallVector<Instruction *, 32> CandidatesForRemoval;
   Value *Cursor = I->getOperand(0), *EarliestReplacement = nullptr;
 
   const auto *IVTy = cast<VectorType>(I->getType());
 
   // Walk the chain of conversions.
   while (Cursor) {
     // If the type of the cursor has fewer lanes than the final result, zeroing

llvm/test/CodeGen/AArch64/sve-intrinsic-opts-reinterpret.ll

br_phi_c:
   br label %join
 
 join:
   %pg = phi <vscale x 16 x i1> [ %a1, %br_phi_a ], [ %b1, %br_phi_b ], [ %c1, %br_phi_c ]
   %pg1 = tail call <vscale x 2 x i1> @llvm.aarch64.sve.convert.from.svbool.nxv2i1(<vscale x 16 x i1> %pg)
   ret <vscale x 16 x i1> %pg
 }
 
+define <vscale x 2 x i1> @reinterpret_zero() {
+; OPT-LABEL:  @reinterpret_zero(
+; OPT: 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 16 x i1> @llvm.aarch64.sve.convert.to.svbool.nxv8i1(<vscale x 8 x i1>)
 declare <vscale x 16 x i1> @llvm.aarch64.sve.convert.to.svbool.nxv4i1(<vscale x 4 x i1>)
 declare <vscale x 16 x i1> @llvm.aarch64.sve.convert.to.svbool.nxv2i1(<vscale x 2 x i1>)
 declare <vscale x 8 x i1> @llvm.aarch64.sve.convert.from.svbool.nxv8i1(<vscale x 16 x i1>)
 declare <vscale x 4 x i1> @llvm.aarch64.sve.convert.from.svbool.nxv4i1(<vscale x 16 x i1>)
 declare <vscale x 2 x i1> @llvm.aarch64.sve.convert.from.svbool.nxv2i1(<vscale x 16 x i1>)