[llvm] [ExpandVectorPredication] Be more precise reporting changes (PR #102313)

[Roger Ferrer Ibáñez via llvm-commits]

https://github.com/rofirrim created https://github.com/llvm/llvm-project/pull/102313

This is used by PreISelIntrinsicLowering. With this change, PreISelIntrinsicLowering does not have to assume that there were changes just because we encountered a VP intrinsic.

>From 61ccb24a9be3d451dd686e9a79957f54402624c8 Mon Sep 17 00:00:00 2001
From: Roger Ferrer Ibanez <rofirrim at gmail.com>
Date: Tue, 6 Aug 2024 21:13:37 +0200
Subject: [PATCH] [ExpandVectorPredication] Be more precise reporting changes

This is used by PreISelIntrinsicLowering. With this change,
PreISelIntrinsicLowering does not have to assume that there were
changes just because we encountered a VP intrinsic.
---
 llvm/lib/CodeGen/ExpandVectorPredication.cpp  | 41 ++++++++++++-------
 llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp |  4 +-
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/llvm/lib/CodeGen/ExpandVectorPredication.cpp b/llvm/lib/CodeGen/ExpandVectorPredication.cpp
index 5ffdbcda93e75..dab5958355e06 100644
--- a/llvm/lib/CodeGen/ExpandVectorPredication.cpp
+++ b/llvm/lib/CodeGen/ExpandVectorPredication.cpp
@@ -160,11 +160,15 @@ struct CachingVPExpander {
   Value *convertEVLToMask(IRBuilder<> &Builder, Value *EVLParam,
                           ElementCount ElemCount);
 
-  Value *foldEVLIntoMask(VPIntrinsic &VPI);
+  /// If needed, folds the EVL in the mask operand and discards the EVL
+  /// parameter. Returns a pair of the value of the intrinsic after the change
+  /// (if any) and whether the mask was actually folded.
+  std::pair<Value *, bool> foldEVLIntoMask(VPIntrinsic &VPI);
 
   /// "Remove" the %evl parameter of \p PI by setting it to the static vector
-  /// length of the operation.
-  void discardEVLParameter(VPIntrinsic &PI);
+  /// length of the operation. Returns true if the %evl (if any) was effectively
+  /// changed.
+  bool discardEVLParameter(VPIntrinsic &PI);
 
   /// Lower this VP binary operator to a unpredicated binary operator.
   Value *expandPredicationInBinaryOperator(IRBuilder<> &Builder,
@@ -206,6 +210,9 @@ struct CachingVPExpander {
   CachingVPExpander(const TargetTransformInfo &TTI)
       : TTI(TTI), UsingTTIOverrides(anyExpandVPOverridesSet()) {}
 
+  /// Applies the legalization strategy for this intrinsic. Returns true
+  /// if any change happened as part of executing the legalization strategy.
+  /// Returns false if the intrinsic is legal.
   bool expandVectorPredication(VPIntrinsic &VPI);
 };
 
@@ -645,15 +652,15 @@ Value *CachingVPExpander::expandPredicationInComparison(IRBuilder<> &Builder,
   return NewCmp;
 }
 
-void CachingVPExpander::discardEVLParameter(VPIntrinsic &VPI) {
+bool CachingVPExpander::discardEVLParameter(VPIntrinsic &VPI) {
   LLVM_DEBUG(dbgs() << "Discard EVL parameter in " << VPI << "\n");
 
   if (VPI.canIgnoreVectorLengthParam())
-    return;
+    return false;
 
   Value *EVLParam = VPI.getVectorLengthParam();
   if (!EVLParam)
-    return;
+    return false;
 
   ElementCount StaticElemCount = VPI.getStaticVectorLength();
   Value *MaxEVL = nullptr;
@@ -672,16 +679,17 @@ void CachingVPExpander::discardEVLParameter(VPIntrinsic &VPI) {
     MaxEVL = ConstantInt::get(Int32Ty, StaticElemCount.getFixedValue(), false);
   }
   VPI.setVectorLengthParam(MaxEVL);
+  return true;
 }
 
-Value *CachingVPExpander::foldEVLIntoMask(VPIntrinsic &VPI) {
+std::pair<Value *, bool> CachingVPExpander::foldEVLIntoMask(VPIntrinsic &VPI) {
   LLVM_DEBUG(dbgs() << "Folding vlen for " << VPI << '\n');
 
   IRBuilder<> Builder(&VPI);
 
   // Ineffective %evl parameter and so nothing to do here.
   if (VPI.canIgnoreVectorLengthParam())
-    return &VPI;
+    return {&VPI, false};
 
   // Only VP intrinsics can have an %evl parameter.
   Value *OldMaskParam = VPI.getMaskParam();
@@ -704,7 +712,7 @@ Value *CachingVPExpander::foldEVLIntoMask(VPIntrinsic &VPI) {
          "transformation did not render the evl param ineffective!");
 
   // Reassess the modified instruction.
-  return &VPI;
+  return {&VPI, true};
 }
 
 Value *CachingVPExpander::expandPredication(VPIntrinsic &VPI) {
@@ -812,16 +820,22 @@ bool CachingVPExpander::expandVectorPredication(VPIntrinsic &VPI) {
   auto Strategy = getVPLegalizationStrategy(VPI);
   sanitizeStrategy(VPI, Strategy);
 
+  bool Changed = false;
+
   // Transform the EVL parameter.
   switch (Strategy.EVLParamStrategy) {
   case VPLegalization::Legal:
     break;
   case VPLegalization::Discard:
-    discardEVLParameter(VPI);
+    if (discardEVLParameter(VPI))
+      Changed = true;
     break;
   case VPLegalization::Convert:
-    if (foldEVLIntoMask(VPI))
+    if (auto [NewVPI, Folded] = foldEVLIntoMask(VPI); Folded) {
+      (void)NewVPI;
+      Changed = true;
       ++NumFoldedVL;
+    }
     break;
   }
 
@@ -834,13 +848,12 @@ bool CachingVPExpander::expandVectorPredication(VPIntrinsic &VPI) {
   case VPLegalization::Convert:
     if (Value *V = expandPredication(VPI); V != &VPI) {
       ++NumLoweredVPOps;
-      // Return true if and only if the intrinsic was actually removed.
-      return true;
+      Changed = true;
     }
     break;
   }
 
-  return false;
+  return Changed;
 }
 } // namespace
 
diff --git a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
index 0d3dd650b8ee6..6418c54f0b18f 100644
--- a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -357,14 +357,12 @@ bool PreISelIntrinsicLowering::lowerIntrinsics(Module &M) const {
 #define BEGIN_REGISTER_VP_INTRINSIC(VPID, MASKPOS, VLENPOS)                    \
   case Intrinsic::VPID:
 #include "llvm/IR/VPIntrinsics.def"
-      forEachCall(F, [&](CallInst *CI) {
+      Changed |= forEachCall(F, [&](CallInst *CI) {
         Function *Parent = CI->getParent()->getParent();
         const TargetTransformInfo &TTI = LookupTTI(*Parent);
         auto *VPI = cast<VPIntrinsic>(CI);
         return expandVectorPredicationIntrinsic(*VPI, TTI);
       });
-      // Not all intrinsics are removed, but the code is changed in any case.
-      Changed = true;
       break;
     case Intrinsic::objc_autorelease:
       Changed |= lowerObjCCall(F, "objc_autorelease");