IPSCCP crashes with vscale

[nunoplopes]

opt -passes=ipsccp file.ll crashes on:

define ptr @fn(ptr %a, i64 %idx) {
  %a2 = getelementptr inbounds [4 x <vscale x 2 x i32>], ptr %a, i64 1
  ret ptr %a2
}

opt: include/llvm/Support/TypeSize.h:183: constexpr llvm::details::FixedOrScalableQuantity<LeafTy, ValueTy>::ScalarTy llvm::details::FixedOrScalableQuantity<LeafTy, ValueTy>::getFixedValue() const [with LeafTy = llvm::TypeSize; ValueTy = long unsigned int; ScalarTy = long unsigned int]: Assertion `(!isScalable() || isZero()) && "Request for a fixed element count on a scalable object"' failed.

#7  llvm::TargetTransformInfoImplCRTPBase<(anonymous namespace)::NoTTIImpl>::getGEPCost(llvm::Type*, llvm::Value const*, llvm::ArrayRef<llvm::Value const*>, llvm::Type*, llvm::TargetTransformInfo::TargetCostKind)
#8  llvm::TargetTransformInfo::getInstructionCost(llvm::User const*, llvm::ArrayRef<llvm::Value const*>, llvm::TargetTransformInfo::TargetCostKind) const
#9  llvm::CodeMetrics::analyzeBasicBlock(llvm::BasicBlock const*, llvm::TargetTransformInfo const&, llvm::SmallPtrSetImpl<llvm::Value const*> const&, bool)
#10 llvm::FunctionSpecializer::run()
#11 runIPSCCP(llvm::Module&, llvm::DataLayout const&, llvm::AnalysisManager<llvm::Function>*, std::function<llvm::TargetLibraryInfo const& (llvm::Function&)>, std::function<llvm::TargetTransformInfo& (llvm::Function&)>, std::function<llvm::AssumptionCache& (llvm::Function&)>, std::function<llvm::DominatorTree& (llvm::Function&)>, std::function<llvm::BlockFrequencyInfo& (llvm::Function&)>, bool)

[nikic]

Looks like fallout from allowing vscale in arrays, not really IPSCCP specific.

[antoniofrighetto]

Considering how the cost model of a GEP is currently computed when indexing into a vector of kind [4 x <2 x i32>] (cost: TTI::TCC_Basic), and being vscale unknown at compile-time, we might continue bailing out when dealing with arrays of scalable type too? Could ever this turn out to be TCC_Expensive if vscale is not known?

--- a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
+++ b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
@@ -1021,7 +1021,10 @@ public:
       } else {
         // If this operand is a scalable type, bail out early.
         // TODO: handle scalable vectors
-        if (isa<ScalableVectorType>(TargetType))
+        Type *ElementType = TargetType;
+        if (auto *ATy = dyn_cast<ArrayType>(TargetType))
+          ElementType = ATy->getElementType();
+        if (isa<ScalableVectorType>(ElementType))
           return TTI::TCC_Basic;
         int64_t ElementSize =
             DL.getTypeAllocSize(GTI.getIndexedType()).getFixedValue();

cc/ @paulwalker-arm, @rengolin.

[rengolin]

being vscale unknown at compile-time, we might continue bailing out when dealing with arrays of scalable type too?

Sounds like the safest approach, for now at least.

Could ever this turn out to be TCC_Expensive if vscale is not known?

The vscale is generally assumed to be unknown at compile time. Your diff needs to continue applying to TargetType as well as ElementType to have an effect without changing the behaviour elsewhere.

If you end up doing that as a workaround, make sure to update the TODO to also include "handle arrays of scalable vectors".

[antoniofrighetto]

Sounds good, although considering that the vscale is unknown, I wonder if it would be more appropriate to compute the GEP cost similarly to what is being already done for fixed-size vectors. @paulwalker-arm, feel free to let me know if you want me to open a patch for it.

[paulwalker-arm]

Thanks for looping me in. Given this is a crash with a simple fix I've just pushed it directly (please shout if there's something I've missed). I've mirrored previous fixes and migrated the code to use isScalableTy() so that in theory all scalable (as in scaled by vscale) types are covered. Though, if the getelementptr of scalable vector structs restriction is relaxed, the code will need moving higher.