Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions llvm/lib/Transforms/Vectorize/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ add_llvm_component_library(LLVMVectorize
Vectorize.cpp
VectorCombine.cpp
VPlan.cpp
VPlanAnalysis.cpp
VPlanHCFGBuilder.cpp
VPlanRecipes.cpp
VPlanSLP.cpp
Expand Down
8 changes: 5 additions & 3 deletions llvm/lib/Transforms/Vectorize/VPlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed!

}
};
Expand Down Expand Up @@ -2080,8 +2082,8 @@ class VPCanonicalIVPHIRecipe : public VPHeaderPHIRecipe {
#endif

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

/// Returns true if the recipe only uses the first lane of operand \p Op.
Expand Down
226 changes: 226 additions & 0 deletions llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
//===- 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::inferScalarType(const VPBlendRecipe *R) {
Type *ResTy = inferScalarType(R->getIncomingValue(0));
for (unsigned I = 1, E = R->getNumIncomingValues(); I != E; ++I) {
VPValue *Inc = R->getIncomingValue(I);
assert(inferScalarType(Inc) == ResTy &&
"different types inferred for different incoming values");
CachedTypes[Inc] = ResTy;
}
return ResTy;
}

Type *VPTypeAnalysis::inferScalarType(const VPInstruction *R) {
switch (R->getOpcode()) {
case Instruction::Select: {
Type *ResTy = inferScalarType(R->getOperand(1));
VPValue *OtherV = R->getOperand(2);
assert(inferScalarType(OtherV) == ResTy &&
"different types inferred for different operands");
CachedTypes[OtherV] = ResTy;
return ResTy;
}
case VPInstruction::FirstOrderRecurrenceSplice:
return inferScalarType(R->getOperand(0));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

infer and/or cache type of operand 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks!

default:
break;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

llvm_unreachable("Unhandled opcode!");
}

Type *VPTypeAnalysis::inferScalarType(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::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 = inferScalarType(R->getOperand(0));
assert(ResTy == inferScalarType(R->getOperand(1)) &&
"types for both operands must match for binary op");
CachedTypes[R->getOperand(1)] = ResTy;
return ResTy;
}
case Instruction::FNeg:
case Instruction::Freeze:
return inferScalarType(R->getOperand(0));
default:
break;
}

// Type inferrence not implemented for opcode.
LLVM_DEBUG(dbgs() << "LV: Found unhandled opcode: "
<< Instruction::getOpcodeName(Opcode));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

llvm_unreachable("Unhandled opcode!");
}

Type *VPTypeAnalysis::inferScalarType(const VPWidenCallRecipe *R) {
auto &CI = *cast<CallInst>(R->getUnderlyingInstr());
return CI.getType();
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an assert, thanks!


return cast<LoadInst>(&R->getIngredient())->getType();
}

Type *VPTypeAnalysis::inferScalarType(const VPWidenSelectRecipe *R) {
Type *ResTy = inferScalarType(R->getOperand(1));
VPValue *OtherV = R->getOperand(2);
assert(inferScalarType(OtherV) == ResTy &&
"different types inferred for different operands");
CachedTypes[OtherV] = ResTy;
return ResTy;
}

Type *VPTypeAnalysis::inferScalarType(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::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: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the inferred type always i1 for ICmp/FCmp?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Type *ResTy = inferScalarType(R->getOperand(0));
assert(ResTy == inferScalarType(R->getOperand(1)) &&
"inferred types for operands of binary op don't match");
CachedTypes[R->getOperand(1)] = ResTy;
return ResTy;
}
case Instruction::Trunc:
case Instruction::SExt:
case Instruction::ZExt:
case Instruction::FPExt:
case Instruction::FPTrunc:
case Instruction::ExtractValue:
return R->getUnderlyingInstr()->getType();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fall through?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks!

case Instruction::Freeze:
case Instruction::FNeg:
return inferScalarType(R->getOperand(0));
case Instruction::Load:
return cast<LoadInst>(R->getUnderlyingInstr())->getType();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it matter that the underlying value is a LoadInst?

Also supply the type of values being stored?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it matter that the underlying value is a LoadInst?

It's an extra consistency check.

Also supply the type of values being stored?
This is bottom up, so I think the store case should not be reachable ATM

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interface returns the type of the defined value of the inputs (the public interface computes the type for a VPValue). As stores won't define a result, no type can be queried for them. Does that make sense?

default:
break;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switches with return/unreachable cases/default should be handled consistently.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be handled consistently now, by sinking unreachable outside.

}

llvm_unreachable("Unhandled instruction");
}

Type *VPTypeAnalysis::inferScalarType(const VPValue *V) {
if (Type *CachedTy = CachedTypes.lookup(V))
return CachedTy;

Type *ResultTy = nullptr;
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:
// Handle header phi recipes, except VPWienIntOrFpInduction which needs
// special handling due it being possibly truncated.
ResultTy = cast<VPHeaderPHIRecipe>(Def)
->getStartValue()
->getLiveInIRValue()
->getType();
break;
case VPDef::VPWidenIntOrFpInductionSC:
ResultTy = cast<VPWidenIntOrFpInductionRecipe>(Def)->getScalarType();
break;
case VPDef::VPPredInstPHISC:
case VPDef::VPScalarIVStepsSC:
case VPDef::VPWidenPHISC:
ResultTy = inferScalarType(Def->getOperand(0));
break;
case VPDef::VPBlendSC:
ResultTy = inferScalarType(cast<VPBlendRecipe>(Def));
break;
case VPDef::VPInstructionSC:
ResultTy = inferScalarType(cast<VPInstruction>(Def));
break;
case VPDef::VPInterleaveSC:
// TODO: Use info from interleave group.
ResultTy = V->getUnderlyingValue()->getType();
break;
case VPDef::VPReplicateSC:
ResultTy = inferScalarType(cast<VPReplicateRecipe>(Def));
break;
case VPDef::VPWidenSC:
ResultTy = inferScalarType(cast<VPWidenRecipe>(Def));
break;
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;
}
61 changes: 61 additions & 0 deletions llvm/lib/Transforms/Vectorize/VPlanAnalysis.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
//===- 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-inference for VPValues.
/// 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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extended comment, thanks!

/// Note that the analysis caches the infered types. A new analysis object must
/// be constructed once a VPlan has been modified in a way that invalidates any
/// of the previously infered types.
class VPTypeAnalysis {
DenseMap<const VPValue *, Type *> CachedTypes;
LLVMContext &Ctx;

Type *inferScalarType(const VPBlendRecipe *R);
Type *inferScalarType(const VPInstruction *R);
Type *inferScalarType(const VPWidenCallRecipe *R);
Type *inferScalarType(const VPWidenRecipe *R);
Type *inferScalarType(const VPWidenIntOrFpInductionRecipe *R);
Type *inferScalarType(const VPWidenMemoryInstructionRecipe *R);
Type *inferScalarType(const VPWidenSelectRecipe *R);
Type *inferScalarType(const VPReplicateRecipe *R);

public:
VPTypeAnalysis(LLVMContext &Ctx) : Ctx(Ctx) {}

/// Infer the type of \p V. Returns the scalar type of \p V.
Type *inferScalarType(const VPValue *V);
};

} // end namespace llvm

#endif // LLVM_TRANSFORMS_VECTORIZE_VPLANANALYSIS_H
13 changes: 13 additions & 0 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -738,7 +739,19 @@ void VPWidenRecipe::execute(VPTransformState &State) {
<< Instruction::getOpcodeName(Opcode));
llvm_unreachable("Unhandled instruction!");
} // end of switch.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do!

for (unsigned Part = 0; Part < State.UF; ++Part) {
assert(VectorType::get(A.inferScalarType(getVPSingleValue()), State.VF) ==
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VPWidenRecipe inherits from VPValue, suffice to use this instead of getVPSingleValue() (which may in general return null)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

State.get(this, Part)->getType() &&
"infered type and type from generated instructions do not match");
}
#endif
}

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void VPWidenRecipe::print(raw_ostream &O, const Twine &Indent,
VPSlotTracker &SlotTracker) const {
Expand Down