[VPlan] Add initial anlysis to infer scalar type of VPValues. #69013

fhahn · 2023-10-13T17:34:05Z

This patch adds initial type inferrence for VPValues. It infers the scalar type of a VPValue, by bottom-up traversing through defining recipes until root nodes with known types are reached (e.g. live-ins or memory recipes). The types are then propagated top down through operations.

This is intended as building block for a VPlan-based cost model, which will need access to type information for VPValues/recipes.

Initial testing is done by asserting the inferred type matches the type of the result value generated for a widen recipe.

This patch adds initial type inferrence for VPValues. It infers the scalar type of a VPValue, by bottom-up traversing through defining recipes until root nodes with known types are reached (e.g. live-ins or memory recipes). The types are then propagated top down through operations. This is intended as building block for a VPlan-based cost model, which will need access to type information for VPValues/recipes. Initial testing is done by asserting the inferred type matches the type of the result value generated for a widen recipe.

fhahn · 2023-10-13T17:34:38Z

This is a first patch to enable basic type analysis for VPlan, to be used for cost-modeling: #67934

llvmbot · 2023-10-13T17:35:07Z

@llvm/pr-subscribers-llvm-transforms

Author: Florian Hahn (fhahn)

Changes

This patch adds initial type inferrence for VPValues. It infers the scalar type of a VPValue, by bottom-up traversing through defining recipes until root nodes with known types are reached (e.g. live-ins or memory recipes). The types are then propagated top down through operations.

This is intended as building block for a VPlan-based cost model, which will need access to type information for VPValues/recipes.

Initial testing is done by asserting the inferred type matches the type of the result value generated for a widen recipe.

Full diff: https://github.com/llvm/llvm-project/pull/69013.diff

5 Files Affected:

(modified) llvm/lib/Transforms/Vectorize/CMakeLists.txt (+1)
(modified) llvm/lib/Transforms/Vectorize/VPlan.h (+5-3)
(added) llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp (+225)
(added) llvm/lib/Transforms/Vectorize/VPlanAnalysis.h (+56)
(modified) llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp (+12)

diff --git a/llvm/lib/Transforms/Vectorize/CMakeLists.txt b/llvm/lib/Transforms/Vectorize/CMakeLists.txt
index 998dfd956575d3c..9674094024b9ec7 100644
--- a/llvm/lib/Transforms/Vectorize/CMakeLists.txt
+++ b/llvm/lib/Transforms/Vectorize/CMakeLists.txt
@@ -6,6 +6,7 @@ add_llvm_component_library(LLVMVectorize
   Vectorize.cpp
   VectorCombine.cpp
   VPlan.cpp
+  VPlanAnalysis.cpp
   VPlanHCFGBuilder.cpp
   VPlanRecipes.cpp
   VPlanSLP.cpp
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index e65a7ab2cd028ee..ea1f8a5b9d1e9ab 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1167,6 +1167,8 @@ class VPWidenRecipe : public VPRecipeWithIRFlags, public VPValue {
   /// Produce widened copies of all Ingredients.
   void execute(VPTransformState &State) override;
 
+  unsigned getOpcode() const { return Opcode; }
+
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   /// Print the recipe.
   void print(raw_ostream &O, const Twine &Indent,
@@ -1458,7 +1460,7 @@ class VPWidenIntOrFpInductionRecipe : public VPHeaderPHIRecipe {
   bool isCanonical() const;
 
   /// Returns the scalar type of the induction.
-  const Type *getScalarType() const {
+  Type *getScalarType() const {
     return Trunc ? Trunc->getType() : IV->getType();
   }
 };
@@ -2080,7 +2082,7 @@ class VPCanonicalIVPHIRecipe : public VPHeaderPHIRecipe {
 #endif
 
   /// Returns the scalar type of the induction.
-  const Type *getScalarType() const {
+  Type *getScalarType() const {
     return getOperand(0)->getLiveInIRValue()->getType();
   }
 
@@ -2149,7 +2151,7 @@ class VPWidenCanonicalIVRecipe : public VPRecipeBase, public VPValue {
 #endif
 
   /// Returns the scalar type of the induction.
-  const Type *getScalarType() const {
+  Type *getScalarType() const {
     return cast<VPCanonicalIVPHIRecipe>(getOperand(0)->getDefiningRecipe())
         ->getScalarType();
   }
diff --git a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
new file mode 100644
index 000000000000000..088da81f950425c
--- /dev/null
+++ b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
@@ -0,0 +1,225 @@
+//===- VPlanAnalysis.cpp - Various Analyses working on VPlan ----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "VPlanAnalysis.h"
+#include "VPlan.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "vplan"
+
+Type *VPTypeAnalysis::inferType(const VPBlendRecipe *R) {
+  return inferType(R->getIncomingValue(0));
+}
+
+Type *VPTypeAnalysis::inferType(const VPInstruction *R) {
+  switch (R->getOpcode()) {
+  case Instruction::Select:
+    return inferType(R->getOperand(1));
+  case VPInstruction::FirstOrderRecurrenceSplice:
+    return inferType(R->getOperand(0));
+  default:
+    llvm_unreachable("Unhandled instruction!");
+  }
+}
+
+Type *VPTypeAnalysis::inferType(const VPInterleaveRecipe *R) { return nullptr; }
+
+Type *VPTypeAnalysis::inferType(const VPReductionPHIRecipe *R) {
+  return R->getOperand(0)->getLiveInIRValue()->getType();
+}
+
+Type *VPTypeAnalysis::inferType(const VPWidenRecipe *R) {
+  unsigned Opcode = R->getOpcode();
+  switch (Opcode) {
+  case Instruction::ICmp:
+  case Instruction::FCmp:
+    return IntegerType::get(Ctx, 1);
+  case Instruction::UDiv:
+  case Instruction::SDiv:
+  case Instruction::SRem:
+  case Instruction::URem:
+  case Instruction::Add:
+  case Instruction::FAdd:
+  case Instruction::Sub:
+  case Instruction::FSub:
+  case Instruction::FNeg:
+  case Instruction::Mul:
+  case Instruction::FMul:
+  case Instruction::FDiv:
+  case Instruction::FRem:
+  case Instruction::Shl:
+  case Instruction::LShr:
+  case Instruction::AShr:
+  case Instruction::And:
+  case Instruction::Or:
+  case Instruction::Xor: {
+    Type *ResTy = inferType(R->getOperand(0));
+    if (Opcode != Instruction::FNeg) {
+      assert(ResTy == inferType(R->getOperand(1)));
+      CachedTypes[R->getOperand(1)] = ResTy;
+    }
+    return ResTy;
+  }
+  case Instruction::Freeze:
+    return inferType(R->getOperand(0));
+  default:
+    // This instruction is not vectorized by simple widening.
+    //    LLVM_DEBUG(dbgs() << "LV: Found an unhandled instruction: " << I);
+    llvm_unreachable("Unhandled instruction!");
+  }
+
+  return nullptr;
+}
+
+Type *VPTypeAnalysis::inferType(const VPWidenCallRecipe *R) {
+  auto &CI = *cast<CallInst>(R->getUnderlyingInstr());
+  return CI.getType();
+}
+
+Type *VPTypeAnalysis::inferType(const VPWidenIntOrFpInductionRecipe *R) {
+  return R->getScalarType();
+}
+
+Type *VPTypeAnalysis::inferType(const VPWidenMemoryInstructionRecipe *R) {
+  if (R->isStore())
+    return cast<StoreInst>(&R->getIngredient())->getValueOperand()->getType();
+
+  return cast<LoadInst>(&R->getIngredient())->getType();
+}
+
+Type *VPTypeAnalysis::inferType(const VPWidenSelectRecipe *R) {
+  return inferType(R->getOperand(1));
+}
+
+Type *VPTypeAnalysis::inferType(const VPReplicateRecipe *R) {
+  switch (R->getUnderlyingInstr()->getOpcode()) {
+  case Instruction::Call: {
+    unsigned CallIdx = R->getNumOperands() - (R->isPredicated() ? 2 : 1);
+    return cast<Function>(R->getOperand(CallIdx)->getLiveInIRValue())
+        ->getReturnType();
+  }
+  case Instruction::UDiv:
+  case Instruction::SDiv:
+  case Instruction::SRem:
+  case Instruction::URem:
+  case Instruction::Add:
+  case Instruction::FAdd:
+  case Instruction::Sub:
+  case Instruction::FSub:
+  case Instruction::FNeg:
+  case Instruction::Mul:
+  case Instruction::FMul:
+  case Instruction::FDiv:
+  case Instruction::FRem:
+  case Instruction::Shl:
+  case Instruction::LShr:
+  case Instruction::AShr:
+  case Instruction::And:
+  case Instruction::Or:
+  case Instruction::Xor:
+  case Instruction::ICmp:
+  case Instruction::FCmp: {
+    Type *ResTy = inferType(R->getOperand(0));
+    assert(ResTy == inferType(R->getOperand(1)));
+    CachedTypes[R->getOperand(1)] = ResTy;
+    return ResTy;
+  }
+  case Instruction::Trunc:
+  case Instruction::SExt:
+  case Instruction::ZExt:
+  case Instruction::FPExt:
+  case Instruction::FPTrunc:
+    return R->getUnderlyingInstr()->getType();
+  case Instruction::ExtractValue: {
+    return R->getUnderlyingValue()->getType();
+  }
+  case Instruction::Freeze:
+    return inferType(R->getOperand(0));
+  case Instruction::Load:
+    return cast<LoadInst>(R->getUnderlyingInstr())->getType();
+  case Instruction::Store:
+    return cast<StoreInst>(R->getUnderlyingInstr())
+        ->getValueOperand()
+        ->getType();
+  default:
+    llvm_unreachable("Unhandled instruction");
+  }
+
+  return nullptr;
+}
+
+Type *VPTypeAnalysis::inferType(const VPValue *V) {
+  auto Iter = CachedTypes.find(V);
+  if (Iter != CachedTypes.end())
+    return Iter->second;
+
+  Type *ResultTy = nullptr;
+  if (V->isLiveIn())
+    ResultTy = V->getLiveInIRValue()->getType();
+  else {
+    const VPRecipeBase *Def = V->getDefiningRecipe();
+    switch (Def->getVPDefID()) {
+    case VPDef::VPBlendSC:
+      ResultTy = inferType(cast<VPBlendRecipe>(Def));
+      break;
+    case VPDef::VPCanonicalIVPHISC:
+      ResultTy = cast<VPCanonicalIVPHIRecipe>(Def)->getScalarType();
+      break;
+    case VPDef::VPFirstOrderRecurrencePHISC:
+      ResultTy = Def->getOperand(0)->getLiveInIRValue()->getType();
+      break;
+    case VPDef::VPInstructionSC:
+      ResultTy = inferType(cast<VPInstruction>(Def));
+      break;
+    case VPDef::VPInterleaveSC:
+      ResultTy = V->getUnderlyingValue()
+                     ->getType(); // inferType(cast<VPInterleaveRecipe>(Def));
+      break;
+    case VPDef::VPPredInstPHISC:
+      ResultTy = inferType(Def->getOperand(0));
+      break;
+    case VPDef::VPReductionPHISC:
+      ResultTy = inferType(cast<VPReductionPHIRecipe>(Def));
+      break;
+    case VPDef::VPReplicateSC:
+      ResultTy = inferType(cast<VPReplicateRecipe>(Def));
+      break;
+    case VPDef::VPScalarIVStepsSC:
+      return inferType(Def->getOperand(0));
+      break;
+    case VPDef::VPWidenSC:
+      ResultTy = inferType(cast<VPWidenRecipe>(Def));
+      break;
+    case VPDef::VPWidenPHISC:
+      return inferType(Def->getOperand(0));
+    case VPDef::VPWidenPointerInductionSC:
+      return inferType(Def->getOperand(0));
+    case VPDef::VPWidenCallSC:
+      ResultTy = inferType(cast<VPWidenCallRecipe>(Def));
+      break;
+    case VPDef::VPWidenCastSC:
+      ResultTy = cast<VPWidenCastRecipe>(Def)->getResultType();
+      break;
+    case VPDef::VPWidenGEPSC:
+      ResultTy = PointerType::get(Ctx, 0);
+      break;
+    case VPDef::VPWidenIntOrFpInductionSC:
+      ResultTy = inferType(cast<VPWidenIntOrFpInductionRecipe>(Def));
+      break;
+    case VPDef::VPWidenMemoryInstructionSC:
+      ResultTy = inferType(cast<VPWidenMemoryInstructionRecipe>(Def));
+      break;
+    case VPDef::VPWidenSelectSC:
+      ResultTy = inferType(cast<VPWidenSelectRecipe>(Def));
+      break;
+    }
+  }
+  CachedTypes[V] = ResultTy;
+  return ResultTy;
+}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.h b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.h
new file mode 100644
index 000000000000000..8fcbe9ca99bb4d5
--- /dev/null
+++ b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.h
@@ -0,0 +1,56 @@
+//===- VPlanAnalysis.h - Various Analyses working on VPlan ------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TRANSFORMS_VECTORIZE_VPLANANALYSIS_H
+#define LLVM_TRANSFORMS_VECTORIZE_VPLANANALYSIS_H
+
+#include "llvm/ADT/DenseMap.h"
+
+namespace llvm {
+
+class LLVMContext;
+class VPValue;
+class VPBlendRecipe;
+class VPInterleaveRecipe;
+class VPInstruction;
+class VPReductionPHIRecipe;
+class VPWidenRecipe;
+class VPWidenCallRecipe;
+class VPWidenCastRecipe;
+class VPWidenIntOrFpInductionRecipe;
+class VPWidenMemoryInstructionRecipe;
+struct VPWidenSelectRecipe;
+class VPReplicateRecipe;
+class Type;
+
+/// An analysis for type-inferrence for VPValues.
+class VPTypeAnalysis {
+  DenseMap<const VPValue *, Type *> CachedTypes;
+  LLVMContext &Ctx;
+
+  Type *inferType(const VPBlendRecipe *R);
+  Type *inferType(const VPInstruction *R);
+  Type *inferType(const VPInterleaveRecipe *R);
+  Type *inferType(const VPWidenCallRecipe *R);
+  Type *inferType(const VPReductionPHIRecipe *R);
+  Type *inferType(const VPWidenRecipe *R);
+  Type *inferType(const VPWidenIntOrFpInductionRecipe *R);
+  Type *inferType(const VPWidenMemoryInstructionRecipe *R);
+  Type *inferType(const VPWidenSelectRecipe *R);
+  Type *inferType(const VPReplicateRecipe *R);
+
+public:
+  VPTypeAnalysis(LLVMContext &Ctx) : Ctx(Ctx) {}
+
+  /// Infer the type of \p V. Returns the scalar type of \p V.
+  Type *inferType(const VPValue *V);
+};
+
+} // end namespace llvm
+
+#endif // LLVM_TRANSFORMS_VECTORIZE_VPLANANALYSIS_H
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 2a1213a98095907..b616abddb00f99a 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -12,6 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "VPlan.h"
+#include "VPlanAnalysis.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Twine.h"
@@ -738,7 +739,18 @@ void VPWidenRecipe::execute(VPTransformState &State) {
                       << Instruction::getOpcodeName(Opcode));
     llvm_unreachable("Unhandled instruction!");
   } // end of switch.
+
+#if !defined(NDEBUG)
+  // Verify that VPlan type infererrence results agree with the type of the
+  // generated values.
+  VPTypeAnalysis A(State.Builder.GetInsertBlock()->getContext());
+  for (unsigned Part = 0; Part < State.UF; ++Part) {
+    assert(VectorType::get(A.inferType(getVPSingleValue()), State.VF) ==
+           State.get(this, Part)->getType());
+  }
+#endif
 }
+
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 void VPWidenRecipe::print(raw_ostream &O, const Twine &Indent,
                           VPSlotTracker &SlotTracker) const {

arcbbb · 2023-10-17T09:47:01Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+  case Instruction::ICmp:
+  case Instruction::FCmp: {


Is the inferred type always i1 for ICmp/FCmp?

Yes, the type inference is for the scalar type only, which always should be i1 for compares.

Would be good to clarify that the type inferred is that of scalars/vector-elements. inferScalarType?

Renamed, thanks!

ICmp and FCmp should return Type::getInt1Ty(Context);

I missed this here,. updated, thanks! I also added a few other missing cases here, including GetElementPtr and added the type verification also the scalarizeInstruction to extend coverage.

arcbbb · 2023-10-17T10:00:48Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+  case Instruction::Store:
+    return cast<StoreInst>(R->getUnderlyingInstr())
+        ->getValueOperand()
+        ->getType();


If inferType is used to infer the return type of the recipe, then Instruction::Store becomes unreachable.

Good point, removed, thanks!

ayalz

Nice step forward!

Underlying values are used only when we have to, right? Worth emphasizing somewhere.

ayalz · 2023-10-22T18:06:28Z

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -2080,7 +2082,7 @@ class VPCanonicalIVPHIRecipe : public VPHeaderPHIRecipe {
 #endif

  /// Returns the scalar type of the induction.
-  const Type *getScalarType() const {
+  Type *getScalarType() const {
    return getOperand(0)->getLiveInIRValue()->getType();


Suggested change

return getOperand(0)->getLiveInIRValue()->getType();

return getStartValue()->getLiveInIRValue()->getType();

while we're here

Adjusted, thanks

ayalz · 2023-10-22T18:21:19Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+  }
+}
+
+Type *VPTypeAnalysis::inferType(const VPInterleaveRecipe *R) { return nullptr; }


Remove? Mark unreachable?

Document somewhere what returning null means - no valid type could be inferred?

Removed for now as this is handled in the caller

Null should never be returned, added an assert

ayalz · 2023-10-22T18:21:46Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+Type *VPTypeAnalysis::inferType(const VPInterleaveRecipe *R) { return nullptr; }
+
+Type *VPTypeAnalysis::inferType(const VPReductionPHIRecipe *R) {
+  return R->getOperand(0)->getLiveInIRValue()->getType();


Suggested change

return R->getOperand(0)->getLiveInIRValue()->getType();

return R->getStartValue()->getLiveInIRValue()->getType();

Updated, thanks!

ayalz · 2023-10-22T18:25:42Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+  case Instruction::Or:
+  case Instruction::Xor: {
+    Type *ResTy = inferType(R->getOperand(0));
+    if (Opcode != Instruction::FNeg) {


Deal with FNeg as a separate case, along with Freeze?

Adjusted, thanks!

ayalz · 2023-10-22T18:26:17Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+    return inferType(R->getOperand(0));
+  default:
+    // This instruction is not vectorized by simple widening.
+    //    LLVM_DEBUG(dbgs() << "LV: Found an unhandled instruction: " << I);


Commented out code?

Updated to work with opcode, thanks!

ayalz · 2023-10-22T19:18:48Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+}
+
+Type *VPTypeAnalysis::inferType(const VPWidenSelectRecipe *R) {
+  return inferType(R->getOperand(1));


cache type of operand 2.

Adjusted, thanks!

ayalz · 2023-10-22T19:23:47Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+#define DEBUG_TYPE "vplan"
+
+Type *VPTypeAnalysis::inferType(const VPBlendRecipe *R) {
+  return inferType(R->getIncomingValue(0));


cache types of all other incoming values?

Done, thanks!

ayalz · 2023-10-22T19:24:06Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+Type *VPTypeAnalysis::inferType(const VPInstruction *R) {
+  switch (R->getOpcode()) {
+  case Instruction::Select:
+    return inferType(R->getOperand(1));


cache type of operand 2?

Done, thanks!

ayalz · 2023-10-22T19:31:29Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+      ResultTy = cast<VPCanonicalIVPHIRecipe>(Def)->getScalarType();
+      break;
+    case VPDef::VPFirstOrderRecurrencePHISC:
+      ResultTy = Def->getOperand(0)->getLiveInIRValue()->getType();


Consistency: this was outlined for Reduction PHI below. Handle both inline together?
And most other header phi's, including VPWidenPointerInductionSC, VPCanonicalIVPHIRecipe,
VPWidenIntOrFpInductionRecipe deserves to be treated separately due to its trunc.

Moved together and to the top, thanks!

ayalz · 2023-10-22T19:45:08Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.h

+class VPReplicateRecipe;
+class Type;
+
+/// An analysis for type-inferrence for VPValues.


Worth saying some more, e.g., on-demand cached bottom-up use-def analysis. Rerun if any recipes are changed to invalidate the cache?

Co-authored-by: ayalz <47719489+ayalz@users.noreply.github.com>

fhahn

Latest comments should be addressed, thanks!

Underlying values are used only when we have to, right? Worth emphasizing somewhere.

Yep, I added the info from the patch description to the documentation in VPlanAnalysis.h

fhahn · 2023-10-23T15:28:16Z

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -2080,7 +2082,7 @@ class VPCanonicalIVPHIRecipe : public VPHeaderPHIRecipe {
 #endif

  /// Returns the scalar type of the induction.
-  const Type *getScalarType() const {
+  Type *getScalarType() const {
    return getOperand(0)->getLiveInIRValue()->getType();


Adjusted, thanks

fhahn · 2023-10-23T15:35:20Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+#define DEBUG_TYPE "vplan"
+
+Type *VPTypeAnalysis::inferType(const VPBlendRecipe *R) {
+  return inferType(R->getIncomingValue(0));


Done, thanks!

fhahn · 2023-10-23T15:36:53Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+  case Instruction::Or:
+  case Instruction::Xor: {
+    Type *ResTy = inferType(R->getOperand(0));
+    if (Opcode != Instruction::FNeg) {


Adjusted, thanks!

fhahn · 2023-10-23T15:37:05Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+Type *VPTypeAnalysis::inferType(const VPInstruction *R) {
+  switch (R->getOpcode()) {
+  case Instruction::Select:
+    return inferType(R->getOperand(1));


Done, thanks!

fhahn · 2023-10-23T15:40:19Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+    return inferType(R->getOperand(0));
+  default:
+    // This instruction is not vectorized by simple widening.
+    //    LLVM_DEBUG(dbgs() << "LV: Found an unhandled instruction: " << I);


Updated to work with opcode, thanks!

fhahn · 2023-10-23T16:01:27Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+
+  Type *ResultTy = nullptr;
+  if (V->isLiveIn())
+    ResultTy = V->getLiveInIRValue()->getType();


Done,thanks!

fhahn · 2023-10-23T16:15:07Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+      ResultTy = cast<VPCanonicalIVPHIRecipe>(Def)->getScalarType();
+      break;
+    case VPDef::VPFirstOrderRecurrencePHISC:
+      ResultTy = Def->getOperand(0)->getLiveInIRValue()->getType();


Moved together and to the top, thanks!

fhahn · 2023-10-23T16:16:08Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+      break;
+    case VPDef::VPInterleaveSC:
+      ResultTy = V->getUnderlyingValue()
+                     ->getType(); // inferType(cast<VPInterleaveRecipe>(Def));


Removed, thanks! Also added a TODO to use the info from the interleave group directly.

fhahn · 2023-10-23T16:19:28Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+      ResultTy = inferType(cast<VPReplicateRecipe>(Def));
+      break;
+    case VPDef::VPScalarIVStepsSC:
+      return inferType(Def->getOperand(0));


collected them and also made sure they are added to the cache.

fhahn · 2023-10-23T16:38:27Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+  }
+}
+
+Type *VPTypeAnalysis::inferType(const VPInterleaveRecipe *R) { return nullptr; }


Null should never be returned, added an assert

github-actions · 2023-10-23T16:52:47Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 8e2bd05c4e86834a318ef2279e271f0769be4988 4ed3b6dfe165aec75f4da8735d392c7aa7ee0822 -- llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp llvm/lib/Transforms/Vectorize/VPlanAnalysis.h llvm/lib/Transforms/Vectorize/VPlan.h llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
index f2e73ca4f63a..27794601fa45 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
@@ -166,12 +166,12 @@ Type *VPTypeAnalysis::inferScalarType(const VPValue *V) {
   if (V->isLiveIn())
     return V->getLiveInIRValue()->getType();
 
-    const VPRecipeBase *Def = V->getDefiningRecipe();
-    switch (Def->getVPDefID()) {
-    case VPDef::VPCanonicalIVPHISC:
-    case VPDef::VPFirstOrderRecurrencePHISC:
-    case VPDef::VPReductionPHISC:
-    case VPDef::VPWidenPointerInductionSC:
+  const VPRecipeBase *Def = V->getDefiningRecipe();
+  switch (Def->getVPDefID()) {
+  case VPDef::VPCanonicalIVPHISC:
+  case VPDef::VPFirstOrderRecurrencePHISC:
+  case VPDef::VPReductionPHISC:
+  case VPDef::VPWidenPointerInductionSC:
     // Handle header phi recipes, except VPWienIntOrFpInduction which needs
     // special handling due it being possibly truncated.
     ResultTy = cast<VPHeaderPHIRecipe>(Def)
@@ -179,47 +179,47 @@ Type *VPTypeAnalysis::inferScalarType(const VPValue *V) {
                    ->getLiveInIRValue()
                    ->getType();
     break;
-    case VPDef::VPWidenIntOrFpInductionSC:
+  case VPDef::VPWidenIntOrFpInductionSC:
     ResultTy = cast<VPWidenIntOrFpInductionRecipe>(Def)->getScalarType();
     break;
-    case VPDef::VPPredInstPHISC:
-    case VPDef::VPScalarIVStepsSC:
-    case VPDef::VPWidenPHISC:
+  case VPDef::VPPredInstPHISC:
+  case VPDef::VPScalarIVStepsSC:
+  case VPDef::VPWidenPHISC:
     ResultTy = inferScalarType(Def->getOperand(0));
     break;
-    case VPDef::VPBlendSC:
+  case VPDef::VPBlendSC:
     ResultTy = inferScalarType(cast<VPBlendRecipe>(Def));
     break;
-    case VPDef::VPInstructionSC:
+  case VPDef::VPInstructionSC:
     ResultTy = inferScalarType(cast<VPInstruction>(Def));
     break;
-    case VPDef::VPInterleaveSC:
+  case VPDef::VPInterleaveSC:
     // TODO: Use info from interleave group.
     ResultTy = V->getUnderlyingValue()->getType();
     break;
-    case VPDef::VPReplicateSC:
+  case VPDef::VPReplicateSC:
     ResultTy = inferScalarType(cast<VPReplicateRecipe>(Def));
     break;
-    case VPDef::VPWidenSC:
+  case VPDef::VPWidenSC:
     ResultTy = inferScalarType(cast<VPWidenRecipe>(Def));
     break;
-    case VPDef::VPWidenCallSC:
+  case VPDef::VPWidenCallSC:
     ResultTy = inferScalarType(cast<VPWidenCallRecipe>(Def));
     break;
-    case VPDef::VPWidenCastSC:
-      ResultTy = cast<VPWidenCastRecipe>(Def)->getResultType();
-      break;
-    case VPDef::VPWidenGEPSC:
-      ResultTy = PointerType::get(Ctx, 0);
-      break;
-    case VPDef::VPWidenMemoryInstructionSC:
-      ResultTy = inferScalarType(cast<VPWidenMemoryInstructionRecipe>(Def));
-      break;
-    case VPDef::VPWidenSelectSC:
-      ResultTy = inferScalarType(cast<VPWidenSelectRecipe>(Def));
-      break;
-    }
-    assert(ResultTy && "could not infer type for the given VPValue");
-    CachedTypes[V] = ResultTy;
-    return ResultTy;
+  case VPDef::VPWidenCastSC:
+    ResultTy = cast<VPWidenCastRecipe>(Def)->getResultType();
+    break;
+  case VPDef::VPWidenGEPSC:
+    ResultTy = PointerType::get(Ctx, 0);
+    break;
+  case VPDef::VPWidenMemoryInstructionSC:
+    ResultTy = inferScalarType(cast<VPWidenMemoryInstructionRecipe>(Def));
+    break;
+  case VPDef::VPWidenSelectSC:
+    ResultTy = inferScalarType(cast<VPWidenSelectRecipe>(Def));
+    break;
+  }
+  assert(ResultTy && "could not infer type for the given VPValue");
+  CachedTypes[V] = ResultTy;
+  return ResultTy;
 }

ayalz · 2023-10-23T23:00:27Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+           "different types inferred for different incoming values");
+    CachedTypes[Inc] = ResTy;
+  }
+  return inferScalarType(R->getIncomingValue(0));


Suggested change

return inferScalarType(R->getIncomingValue(0));

return ResTy;

Fixed, thanks

ayalz · 2023-10-23T23:07:42Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+    return inferScalarType(R->getOperand(0));
+  default:
+    llvm_unreachable("Unhandled instruction!");
+  }


Ah, this works w/o return nor unreachable at the end? Very well. Perhaps worth a comment.

Switch below should be consistent, i.e., with an unreachable default but no return at the unreachable end.

I updated it to move the unreachable out of the switch here as well

ayalz · 2023-10-23T23:14:20Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+  case Instruction::ICmp:
+  case Instruction::FCmp: {
+    Type *ResTy = inferScalarType(R->getOperand(0));
+    assert(ResTy == inferScalarType(R->getOperand(1)));


assert missing a message.

Added message, thanks!

ayalz · 2023-10-23T23:14:56Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+  case Instruction::ZExt:
+  case Instruction::FPExt:
+  case Instruction::FPTrunc:
+    return R->getUnderlyingInstr()->getType();


fall through?

Done, thanks!

ayalz · 2023-10-23T23:18:32Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+  case Instruction::Freeze:
+    return inferType(R->getOperand(0));
+  case Instruction::Load:
+    return cast<LoadInst>(R->getUnderlyingInstr())->getType();


Does it matter that the underlying value is a LoadInst?

It's an extra consistency check.

LoadInst deserves an extra consistency check that truncation and extends do not?

Also supply the type of values being stored?

This is bottom up, so I think the store case should not be reachable ATM

Stores cannot be reached indirectly, but one can query their type directly, and one did provide support for querying the type of a widened stores above and interleaved stored below. Treatment should be consistent regardless of how stores are handled (replicated, widened, interleaved). If querying the type of stores is forbidden, it should be documented.

ayalz · 2023-10-23T23:21:24Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+  if (V->isLiveIn())
+    return V->getLiveInIRValue()->getType();
+
+    const VPRecipeBase *Def = V->getDefiningRecipe();


Indent/clang-format?

Applied clang-format to file, as clang-format-diff was missing this, thanks!

ayalz · 2023-10-23T23:26:58Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.h

+/// It infers the scalar type for a given VPValue by bottom-up traversing
+/// through defining recipes until root nodes with known types are reached (e.g.
+/// live-ins or memory recipes). The types are then propagated top down through
+/// operations.


The cache cannot be flushed so any changes made to VPlan after invoking the analysis render it obsolete and a new analysis must be constructed.

Extended comment, thanks!

ayalz · 2023-10-24T08:30:56Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+
+Type *VPTypeAnalysis::inferScalarType(const VPWidenMemoryInstructionRecipe *R) {
+  if (R->isStore())
+    return cast<StoreInst>(&R->getIngredient())->getValueOperand()->getType();


(Independent Thought) Should VPWidenMemoryInstructionRecipe use Underlying instead of Ingredient, consistent with other recipes?

ayalz · 2023-10-24T10:02:32Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+    return V->getLiveInIRValue()->getType();
+
+    const VPRecipeBase *Def = V->getDefiningRecipe();
+    switch (Def->getVPDefID()) {


Would TypeSwitch work better?

I am not sure, I think with TypeSwich we would not be able to use fallthroughs, which reduce the duplication a bit here (I've not used TypeSwitch before, so perhaps I missed a way to handle fallthroughs).

Updated to use typeswitch, helps to reduce some duplication

ayalz · 2023-10-24T10:03:34Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

+  VPTypeAnalysis A(State.Builder.GetInsertBlock()->getContext());
+  for (unsigned Part = 0; Part < State.UF; ++Part) {
+    assert(VectorType::get(A.inferScalarType(getVPSingleValue()), State.VF) ==
+           State.get(this, Part)->getType());


assert is missing a message.

Added, thanks!

arcbbb · 2023-10-25T14:56:44Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+Type *VPTypeAnalysis::inferScalarType(const VPWidenMemoryInstructionRecipe *R) {
+  if (R->isStore())
+    return cast<StoreInst>(&R->getIngredient())->getValueOperand()->getType();


The store doesn't defined a value, should we make it unreachable here or return a void type?

Added an assert, thanks!

fhahn · 2023-10-26T20:09:34Z

The last update switch to using a TypeSwitch as suggested by @ayalz as it helps to reduce some of the duplication.

ayalz · 2023-10-26T23:39:13Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+    return ResTy;
+  }
+  case VPInstruction::FirstOrderRecurrenceSplice:
+    return inferScalarType(R->getOperand(0));


infer and/or cache type of operand 1?

Done, thanks!

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

ayalz · 2023-10-26T23:50:37Z

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -2192,6 +2194,8 @@ class VPDerivedIVRecipe : public VPRecipeBase, public VPValue {
             VPSlotTracker &SlotTracker) const override;
 #endif

+  Type *getScalarType() const { return TruncResultTy; }


Check if TruncResultTy is null, similar to VPWidenIntOrFpInductionRecipe::getScalarType() above?

Moved, thanks!

ayalz · 2023-10-26T23:54:50Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+            Type *T = R->getScalarType();
+            return T ? T : inferScalarType(R->getOperand(0));


(R->getStartValue()) if needed.
Have getScalarType() worry about truncated or first operand type?
Combine with VPWidenIntOrFpInductionRecipe?

Done, thanks!

llvm/lib/Transforms/Vectorize/VPlanAnalysis.h

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

llvm/lib/Transforms/Vectorize/VPlanAnalysis.h

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

ayalz · 2023-10-27T00:06:29Z

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -1458,7 +1460,7 @@ class VPWidenIntOrFpInductionRecipe : public VPHeaderPHIRecipe {
  bool isCanonical() const;

  /// Returns the scalar type of the induction.
-  const Type *getScalarType() const {
+  Type *getScalarType() const {
    return Trunc ? Trunc->getType() : IV->getType();


Can wait for separate subsequent patch: type of all header phi's should arguably be derived from their start value. If it gets truncated, let the start value be truncated in VPlan. Similar for DerivedIV.

ayalz · 2023-10-27T00:12:00Z

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

    Cloned->setName(Instr->getName() + ".cloned");
+#if !defined(NDEBUG)
+    // Verify that VPlan type inference results agree with the type of the


Check should appear in execute, of every recipe (fine to add others later), but easier for replicate recipe to have it here, once?

Yep it is simpler here, as the current structure of VPReplicateRecipe::execute uses earlier exits for different cases

Co-authored-by: ayalz <47719489+ayalz@users.noreply.github.com>

fhahn

Addressed latest comments, thanks!

fhahn · 2023-10-27T10:27:19Z

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

    Cloned->setName(Instr->getName() + ".cloned");
+#if !defined(NDEBUG)
+    // Verify that VPlan type inference results agree with the type of the


Yep it is simpler here, as the current structure of VPReplicateRecipe::execute uses earlier exits for different cases

fhahn · 2023-10-27T10:27:34Z

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -1458,7 +1460,7 @@ class VPWidenIntOrFpInductionRecipe : public VPHeaderPHIRecipe {
  bool isCanonical() const;

  /// Returns the scalar type of the induction.
-  const Type *getScalarType() const {
+  Type *getScalarType() const {
    return Trunc ? Trunc->getType() : IV->getType();


fhahn · 2023-10-27T10:29:53Z

llvm/lib/Transforms/Vectorize/VPlan.h

@@ -2192,6 +2194,8 @@ class VPDerivedIVRecipe : public VPRecipeBase, public VPValue {
             VPSlotTracker &SlotTracker) const override;
 #endif

+  Type *getScalarType() const { return TruncResultTy; }


Moved, thanks!

fhahn · 2023-10-27T10:31:42Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+    return ResTy;
+  }
+  case VPInstruction::FirstOrderRecurrenceSplice:
+    return inferScalarType(R->getOperand(0));


Done, thanks!

fhahn · 2023-10-27T10:32:02Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+            Type *T = R->getScalarType();
+            return T ? T : inferScalarType(R->getOperand(0));


Done, thanks!

fhahn · 2023-10-27T10:37:27Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

+  // generated values.
+  VPTypeAnalysis A(State.Builder.GetInsertBlock()->getContext());
+  for (unsigned Part = 0; Part < State.UF; ++Part) {
+    assert(VectorType::get(A.inferScalarType(getVPSingleValue()), State.VF) ==


Issue was that overloaded function resolution was picking the more specific call inferScalarType(VPWidenRecipe *), not the more general VPValue one. Renamed the recipe specific ones to `inferScalarTypeForRecipe to resolve this

ayalz

This looks fine to me, thanks! Adding a few last nits.

ayalz · 2023-10-27T12:16:53Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+
+  // Type inference not implemented for opcode.
+  LLVM_DEBUG(dbgs() << "LV: Found unhandled opcode: "
+                    << Instruction::getOpcodeName(Opcode));


nit: better dump the whole underlying instruction than/in addition to only its opcode.

nit: can dump them for other unhandled cases of VPInstruction and replicate recipe above and below.

ayalz · 2023-10-27T12:22:27Z

llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp

+                VPReductionPHIRecipe, VPWidenPointerInductionRecipe>(
+              [this](const auto *R) {
+                // Handle header phi recipes, except VPWienIntOrFpInduction
+                // which needs special handling due it being possibly truncated.


nit/TODO: consider inferring/caching type of siblings, e.g., backedge value, here and in cases below.

Added TODO, thanks

ayalz · 2023-10-27T12:32:24Z

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

+#if !defined(NDEBUG)
+  // Verify that VPlan type inference results agree with the type of the
+  // generated values.
+  VPTypeAnalysis A(State.Builder.GetInsertBlock()->getContext());


Caching types is unhelpful and even harmful time-wise if a VPTypeAnalysis is built for every call of inferScalarType().
For validation here it may be fine, but better avoid having it as the only usage example. Worth building one instance to be used across VPlan execution, possibly stored in State?

Fixes a crash after 0c8e5be. Full type inference will be added in #69013

Extra test for #69013

fhahn · 2023-10-27T13:35:55Z

Thanks!

fhahn requested review from rengolin, arcbbb, ayalz and aniragil October 13, 2023 17:34

llvmbot added vectorizers llvm:transforms labels Oct 13, 2023

fhahn mentioned this pull request Oct 13, 2023

[RFC][LV] VPlan-based cost model #67647

Closed

arcbbb reviewed Oct 17, 2023

View reviewed changes

Remove Store case for VPReplicateRecipe, as it is dead code.

30a8968

ayalz reviewed Oct 22, 2023

View reviewed changes

fhahn and others added 3 commits October 23, 2023 17:19

Update llvm/lib/Transforms/Vectorize/VPlanAnalysis.h

58b145e

Co-authored-by: ayalz <47719489+ayalz@users.noreply.github.com>

Update llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

d89b7d8

Co-authored-by: ayalz <47719489+ayalz@users.noreply.github.com>

Address comments

4ed3b6d

fhahn commented Oct 23, 2023

View reviewed changes

Fix formatting.

529805c

ayalz reviewed Oct 24, 2023

View reviewed changes

Address comments

a75c562

arcbbb reviewed Oct 25, 2023

View reviewed changes

fhahn added 3 commits October 25, 2023 19:51

Add assert to rule out stores in inferScalarType

1483f63

Add a few more missed cases, verify types for VPReplicateRecipe codegen.

79bcce9

Use TypeSwitch.

532d75e

ayalz reviewed Oct 27, 2023

View reviewed changes

fhahn and others added 2 commits October 27, 2023 11:24

Apply suggestions from code review

5bbd764

Co-authored-by: ayalz <47719489+ayalz@users.noreply.github.com>

Address latest comments, thanks!

74525ad

fhahn commented Oct 27, 2023

View reviewed changes

ayalz approved these changes Oct 27, 2023

View reviewed changes

fhahn added a commit that referenced this pull request Oct 27, 2023

[VPlan] Handle VPValues without underlying values in getTypeForVPValue.

cff6652

Fixes a crash after 0c8e5be. Full type inference will be added in #69013

Address latest comments.

722cc3d

fhahn added a commit that referenced this pull request Oct 27, 2023

[LV] Add test case to scalarize ptrtoint instructions.

cdc5e00

Extra test for #69013

fhahn merged commit b0b8864 into llvm:main Oct 27, 2023
1 of 3 checks passed

fhahn deleted the vplan-type branch October 27, 2023 13:38

	return getOperand(0)->getLiveInIRValue()->getType();
	return getStartValue()->getLiveInIRValue()->getType();

	return R->getOperand(0)->getLiveInIRValue()->getType();
	return R->getStartValue()->getLiveInIRValue()->getType();

	return inferScalarType(R->getIncomingValue(0));
	return ResTy;

		Type *T = R->getScalarType();
		return T ? T : inferScalarType(R->getOperand(0));

[VPlan] Add initial anlysis to infer scalar type of VPValues. #69013

[VPlan] Add initial anlysis to infer scalar type of VPValues. #69013

Conversation

fhahn commented Oct 13, 2023

fhahn commented Oct 13, 2023

llvmbot commented Oct 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayalz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fhahn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fhahn commented Oct 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment