[mlir] --convert-nvgpu-to-nvvm crashed with assertion failure "expected integer attribute type to equal element type"

git version: b82c629

system: Ubuntu 20.04.6 LTS (Focal Fossa)

reproduced with: mlir-opt --convert-nvgpu-to-nvvm a.mlir

a.mlir:

func.func @func2() {
  %220 = llvm.mlir.constant(0 : index) : i64
  %365 = vector.broadcast %220 : i64 to vector<26xi64>
  return
}

trace:

mlir-opt: /data/bin/llvm-project/mlir/lib/IR/BuiltinAttributes.cpp:961: static mlir::DenseElementsAttr mlir::DenseElementsAttr::get(mlir::ShapedType, llvm::ArrayRef<mlir::Attribute>): Assertion `intAttr.getType() == eltType && "expected integer attribute type to equal element type"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.      Program arguments: /data/bin/llvm-project/build/bin/mlir-opt --convert-nvgpu-to-nvvm a.mlir
 #0 0x000055a00d2e59cf llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/data/bin/llvm-project/build/bin/mlir-opt+0x15ce9cf)
 #1 0x000055a00d2e2a64 SignalHandler(int) Signals.cpp:0:0
 #2 0x00007f8686c71420 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
 #3 0x00007f868675c00b raise (/lib/x86_64-linux-gnu/libc.so.6+0x4300b)
 #4 0x00007f868673b859 abort (/lib/x86_64-linux-gnu/libc.so.6+0x22859)
 #5 0x00007f868673b729 (/lib/x86_64-linux-gnu/libc.so.6+0x22729)
 #6 0x00007f868674cfd6 (/lib/x86_64-linux-gnu/libc.so.6+0x33fd6)
 #7 0x000055a00ff9db61 mlir::DenseElementsAttr::get(mlir::ShapedType, llvm::ArrayRef<mlir::Attribute>) (/data/bin/llvm-project/build/bin/mlir-opt+0x4286b61)
 #8 0x000055a00f284554 mlir::vector::BroadcastOp::fold(mlir::vector::BroadcastOpGenericAdaptor<llvm::ArrayRef<mlir::Attribute>>) (/data/bin/llvm-project/build/bin/mlir-opt+0x356d554)
 #9 0x000055a00f284656 mlir::LogicalResult llvm::detail::UniqueFunctionBase<mlir::LogicalResult, mlir::Operation*, llvm::ArrayRef<mlir::Attribute>, llvm::SmallVectorImpl<mlir::OpFoldResult>&>::CallImpl<mlir::Op<mlir::vector::BroadcastOp, mlir::OpTrait::ZeroRegions, mlir::OpTrait::OneResult, mlir::OpTrait::OneTypedResult<mlir::VectorType>::Impl, mlir::OpTrait::ZeroSuccessors, mlir::OpTrait::OneOperand, mlir::OpTrait::OpInvariants, mlir::ConditionallySpeculatable::Trait, mlir::OpTrait::AlwaysSpeculatableImplTrait, mlir::MemoryEffectOpInterface::Trait>::getFoldHookFn()::'lambda'(mlir::Operation*, llvm::ArrayRef<mlir::Attribute>, llvm::SmallVectorImpl<mlir::OpFoldResult>&) const>(void*, mlir::Operation*, llvm::ArrayRef<mlir::Attribute>, llvm::SmallVectorImpl<mlir::OpFoldResult>&) (/data/bin/llvm-project/build/bin/mlir-opt+0x356d656)
#10 0x000055a00f2c2756 mlir::RegisteredOperationName::Model<mlir::vector::BroadcastOp>::foldHook(mlir::Operation*, llvm::ArrayRef<mlir::Attribute>, llvm::SmallVectorImpl<mlir::OpFoldResult>&) (/data/bin/llvm-project/build/bin/mlir-opt+0x35ab756)
#11 0x000055a01003f642 mlir::Operation::fold(llvm::ArrayRef<mlir::Attribute>, llvm::SmallVectorImpl<mlir::OpFoldResult>&) (/data/bin/llvm-project/build/bin/mlir-opt+0x4328642)
#12 0x000055a01003f988 mlir::Operation::fold(llvm::SmallVectorImpl<mlir::OpFoldResult>&) (/data/bin/llvm-project/build/bin/mlir-opt+0x4328988)
#13 0x000055a00ff7077a mlir::OpBuilder::tryFold(mlir::Operation*, llvm::SmallVectorImpl<mlir::Value>&) (/data/bin/llvm-project/build/bin/mlir-opt+0x425977a)
#14 0x000055a00fedc8db (anonymous namespace)::OperationLegalizer::legalize(mlir::Operation*, mlir::ConversionPatternRewriter&) DialectConversion.cpp:0:0
#15 0x000055a00fedd397 (anonymous namespace)::OperationConverter::convertOperations(llvm::ArrayRef<mlir::Operation*>, llvm::function_ref<void (mlir::Diagnostic&)>) DialectConversion.cpp:0:0
#16 0x000055a00fedf804 mlir::applyPartialConversion(mlir::Operation*, mlir::ConversionTarget const&, mlir::FrozenRewritePatternSet const&, llvm::DenseSet<mlir::Operation*, llvm::DenseMapInfo<mlir::Operation*, void>>*) (/data/bin/llvm-project/build/bin/mlir-opt+0x41c8804)
#17 0x000055a00f63dde2 (anonymous namespace)::ConvertNVGPUToNVVMPass::runOnOperation() NVGPUToNVVM.cpp:0:0
#18 0x000055a00fe36f6e mlir::detail::OpToOpPassAdaptor::run(mlir::Pass*, mlir::Operation*, mlir::AnalysisManager, bool, unsigned int) (/data/bin/llvm-project/build/bin/mlir-opt+0x411ff6e)
#19 0x000055a00fe3744a mlir::detail::OpToOpPassAdaptor::runPipeline(mlir::OpPassManager&, mlir::Operation*, mlir::AnalysisManager, bool, unsigned int, mlir::PassInstrumentor*, mlir::PassInstrumentation::PipelineParentInfo const*) (/data/bin/llvm-project/build/bin/mlir-opt+0x412044a)
#20 0x000055a00fe37c74 mlir::PassManager::run(mlir::Operation*) (/data/bin/llvm-project/build/bin/mlir-opt+0x4120c74)
#21 0x000055a00fe2853b performActions(llvm::raw_ostream&, std::shared_ptr<llvm::SourceMgr> const&, mlir::MLIRContext*, mlir::MlirOptMainConfig const&) MlirOptMain.cpp:0:0
#22 0x000055a00fe299f4 processBuffer(llvm::raw_ostream&, std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer>>, mlir::MlirOptMainConfig const&, mlir::DialectRegistry&, llvm::ThreadPool*) MlirOptMain.cpp:0:0
#23 0x000055a00fe29ba4 mlir::LogicalResult llvm::function_ref<mlir::LogicalResult (std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer>>, llvm::raw_ostream&)>::callback_fn<mlir::MlirOptMain(llvm::raw_ostream&, std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer>>, mlir::DialectRegistry&, mlir::MlirOptMainConfig const&)::'lambda'(std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer>>, llvm::raw_ostream&)>(long, std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer>>, llvm::raw_ostream&) MlirOptMain.cpp:0:0
#24 0x000055a00ff265b4 mlir::splitAndProcessBuffer(std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer>>, llvm::function_ref<mlir::LogicalResult (std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer>>, llvm::raw_ostream&)>, llvm::raw_ostream&, bool, bool) (/data/bin/llvm-project/build/bin/mlir-opt+0x420f5b4)
#25 0x000055a00fe22e77 mlir::MlirOptMain(llvm::raw_ostream&, std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer>>, mlir::DialectRegistry&, mlir::MlirOptMainConfig const&) (/data/bin/llvm-project/build/bin/mlir-opt+0x410be77)
#26 0x000055a00fe29ecb mlir::MlirOptMain(int, char**, llvm::StringRef, mlir::DialectRegistry&) (/data/bin/llvm-project/build/bin/mlir-opt+0x4112ecb)
#27 0x000055a00d21db4b main (/data/bin/llvm-project/build/bin/mlir-opt+0x1506b4b)
#28 0x00007f868673d083 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24083)
#29 0x000055a00d2b62fe _start (/data/bin/llvm-project/build/bin/mlir-opt+0x159f2fe)
Aborted (core dumped)

[joker-eph]

%220 = llvm.mlir.constant(0 : index) : i64

I suspect this should be invalid IR: the constant should match the return type, @ftynse ?

[ftynse]

I suspect this should be invalid IR: the constant should match the return type, @ftynse ?

It is valid, that's why there are two types printed: one is the type of the attribute, another is the type of the produced value. This is the default form of lowering arith.constant 0 : index to the LLVM dialect, for example. This goes back to the discussion of typed attributes and the fact that index doesn't have a known bitwidth until very late (translation to LLVM IR has a datalayout) so the conversion can't change the attribute itself.

IMO, the folder of llvm.mlir.constant should be more careful and not fold in case of type mismatch.

[joker-eph]

This goes back to the discussion of typed attributes and the fact that index doesn't have a known bitwidth until very late (translation to LLVM IR has a datalayout) so the conversion can't change the attribute itself.

Uh but the conversion somehow is able to change the SSA value type to i64? Why would it be different for the Attribute?

[joker-eph]

IMO, the folder of llvm.mlir.constant should be more careful and not fold in case of type mismatch.

It is the vector.broadcast op here actually:

llvm-project/mlir/lib/Dialect/Vector/IR/VectorOps.cpp

Lines 2258 to 2267 in cf3ac96

	if (getSourceType() == getResultVectorType())
	return getSource();
	if (!adaptor.getSource())
	return {};
	auto vectorType = getResultVectorType();
	if (llvm::isa<IntegerAttr, FloatAttr>(adaptor.getSource()))
	return DenseElementsAttr::get(vectorType, adaptor.getSource());
	if (auto attr = llvm::dyn_cast<SplatElementsAttr>(adaptor.getSource()))
	return DenseElementsAttr::get(vectorType, attr.getSplatValue<Attribute>());
	return {};

I'm not even sure if it is a good idea to fold a broadcast, but contrary to tensors, vectors may be always small enough?

[joker-eph]

I am not sure what the expectation of the fold API should be though: do we expect fold to be provided with an attribute that has a different type than the matching SSA operand?
(@jpienaar @Mogball )

[ftynse]

Uh but the conversion somehow is able to change the SSA value type to i64? Why would it be different for the Attribute?

It's all hazy for me now, but I remember this was a long and complex discussion confounding multiple things including:

• the index type not having a bitwidth so unclear how to store attributes of index type (we were/are assuming that they are at least 64 bits for this purpose);
• the absence of a data layout that would indicate the bitwidth;
• why do we have types on attributes at all?
• there is no mechanism for attribute conversion (unlike types);
• there is no generic way to traverse subelements of an attribute to find all types and convert those (unlike operations where we have access to all result types);
• we still need to support array/elements attributes as values of constants for translation;
  and I'm probably forgetting something.

I think having a constant folder give an attribute with a different type than the type of the result value is surprising, so I attempted a fix in the LLVM constant folding: #70318. Given that only some attributes have types, we may also decide that folders should not assume anything about types of the attributes and be more robust to potential mismatches, at which point we should rather change the vector.broadcast folder.

[jpienaar]

I don't think it's expected no. I think it's similar to canonicalize where the type has to match. I'd expect fold to only return a typed attribute and that type to match the type of the result being folded. In other cases I'd expect fold to fail. I recall there was discussion on potential interpretation gap wrt fold and Attributes, but it would seem like a gap in types if this didn't hold.

[Mogball]

IMO we should change folders to always return a TypedAttr. Conceptually it doesn't make much sense for an op folder to return an attribute without a type.