[HW] Make sure the index type for arrays is at least i1

[pscabot]

When converting bitcast ops, we collect all unpacked integers to cast to a packed integer. If we encounter an array, we create a get operation for the array elements which require the index type. The index type is calculated using lvm::Log2_64_Ceil of the number of elements in the array. If the number of elements was 1 then prior to this fix the index type would be i0, resulting in a crash when lowering to llvm-ir.

So if we have a code like this:

wire [0:0][7:0]  _GEN_29 = '{8'h0};

Which is converted to mlir using ImportVerilog to produce:

%39 = hw.aggregate_constant [0 : i8] {sv.namehint = "_GEN_29"} : !hw.array<1xi8>
...
%99 = hw.bitcast %39 : (!hw.array<1xi8>) -> i8

It generates the following llvm-mlir

%587 = llvm.mlir.constant(0 : i0) : i0
%588 = llvm.zext %587 : i0 to i1
%589 = llvm.getelementptr %68[0, %588] : (!llvm.ptr, i1) -> !llvm.ptr, !llvm.array<1 x i8>
%590 = llvm.load %589 : !llvm.ptr -> i8

Which, when lowering to llvm-ir, causes an assert: /home/ucsdev/pcabot/develop/circt/llvm/llvm/lib/IR/Type.cpp:319: static IntegerType *llvm::IntegerType::get(LLVMContext &, unsigned int): Assertion NumBits >= MIN_INT_BITS && "bitwidth too small"' failed.`

Stack trace:

 #0 0x00005f993a744f88 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/ucsdev/pcabot/develop/circt/llvm/llvm/lib/Support/Unix/Signals.inc:842:13
 #1 0x00005f993a742c93 llvm::sys::RunSignalHandlers() /home/ucsdev/pcabot/develop/circt/llvm/llvm/lib/Support/Signals.cpp:109:18
 #2 0x00005f993a745d11 SignalHandler(int, siginfo_t*, void*) /home/ucsdev/pcabot/develop/circt/llvm/llvm/lib/Support/Unix/Signals.inc:429:38
 #3 0x0000707596a45330 (/lib/x86_64-linux-gnu/libc.so.6+0x45330)
 #4 0x0000707596a9eb2c __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
 #5 0x0000707596a9eb2c __pthread_kill_internal ./nptl/pthread_kill.c:78:10
 #6 0x0000707596a9eb2c pthread_kill ./nptl/pthread_kill.c:89:10
 #7 0x0000707596a4527e raise ./signal/../sysdeps/posix/raise.c:27:6
 #8 0x0000707596a288ff abort ./stdlib/abort.c:81:7
 #9 0x0000707596a2881b _nl_load_domain ./intl/loadmsgcat.c:1177:9
#10 0x0000707596a3b517 (/lib/x86_64-linux-gnu/libc.so.6+0x3b517)
#11 0x00005f993a89a941 llvm::IntegerType::get(llvm::LLVMContext&, unsigned int) /home/ucsdev/pcabot/develop/circt/llvm/llvm/lib/IR/Type.cpp:320:3
#12 0x00005f993c1e4704 mlir::LLVM::detail::TypeToLLVMIRTranslatorImpl::translate(mlir::IntegerType) /home/ucsdev/pcabot/develop/circt/llvm/mlir/lib/Target/LLVMIR/TypeToLLVM.cpp:0:12
#13 0x00005f993c1e4704 auto mlir::LLVM::detail::TypeToLLVMIRTranslatorImpl::translateType(mlir::Type)::'lambda'(auto)::operator()<mlir::IntegerType>(auto) const /home/ucsdev/pcabot/develop/circ
#14 0x00005f993c1e4704 llvm::TypeSwitch<mlir::Type, llvm::Type*>& llvm::TypeSwitch<mlir::Type, llvm::Type*>::Case<mlir::IntegerType, mlir::LLVM::detail::TypeToLLVMIRTranslatorImpl::translateType(mlir::Type)::
#15 0x00005f993c1e4704 mlir::LLVM::detail::TypeToLLVMIRTranslatorImpl::translateType(mlir::Type) /home/ucsdev/pcabot/develop/circt/llvm/mlir/lib/Target/LLVMIR/TypeToLLVM.cpp:73:14
#16 0x00005f993c12f897 convertOperationImpl(mlir::Operation&, llvm::IRBuilderBase&, mlir::LLVM::ModuleTranslation&) /home/ucsdev/pcabot/develop/chips/build/tools/mlir/include/mlir/Dialect/LLVMIR/LLVMConversio
#17 0x00005f993c1c539a llvm::LogicalResult::failed() const /home/ucsdev/pcabot/develop/circt/llvm/llvm/include/llvm/Support/LogicalResult.h:43:43
#18 0x00005f993c1c539a llvm::failed(llvm::LogicalResult) /home/ucsdev/pcabot/develop/circt/llvm/llvm/include/llvm/Support/LogicalResult.h:71:58
#19 0x00005f993c1c539a mlir::LLVM::ModuleTranslation::convertOperation(mlir::Operation&, llvm::IRBuilderBase&, bool) /home/ucsdev/pcabot/develop/circt/llvm/mlir/lib/Target/LLVMIR/ModuleTranslat
#20 0x00005f993c1c5b35 llvm::LogicalResult::failed() const /home/ucsdev/pcabot/develop/circt/llvm/llvm/include/llvm/Support/LogicalResult.h:43:43
#21 0x00005f993c1c5b35 llvm::failed(llvm::LogicalResult) /home/ucsdev/pcabot/develop/circt/llvm/llvm/include/llvm/Support/LogicalResult.h:71:58
#22 0x00005f993c1c5b35 mlir::LLVM::ModuleTranslation::convertBlockImpl(mlir::Block&, bool, llvm::IRBuilderBase&, bool) /home/ucsdev/pcabot/develop/circt/llvm/mlir/lib/Target/LLVMIR/ModuleTransl
#23 0x00005f993c1ca402 mlir::LLVM::ModuleTranslation::convertOneFunction(mlir::LLVM::LLVMFuncOp) /home/ucsdev/pcabot/develop/circt/llvm/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp:1604:16
#24 0x00005f993c1cd174 llvm::LogicalResult::failed() const /home/ucsdev/pcabot/develop/circt/llvm/llvm/include/llvm/Support/LogicalResult.h:43:43
#25 0x00005f993c1cd174 llvm::failed(llvm::LogicalResult) /home/ucsdev/pcabot/develop/circt/llvm/llvm/include/llvm/Support/LogicalResult.h:71:58
#26 0x00005f993c1cd174 mlir::LLVM::ModuleTranslation::convertFunctions() /home/ucsdev/pcabot/develop/circt/llvm/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp:0:0
#27 0x00005f993c1d06bf llvm::LogicalResult::failed() const /home/ucsdev/pcabot/develop/circt/llvm/llvm/include/llvm/Support/LogicalResult.h:43:43
#28 0x00005f993c1d06bf llvm::failed(llvm::LogicalResult) /home/ucsdev/pcabot/develop/circt/llvm/llvm/include/llvm/Support/LogicalResult.h:71:58
#29 0x00005f993c1d06bf mlir::translateModuleToLLVMIR(mlir::Operation*, llvm::LLVMContext&, llvm::StringRef, bool) /home/ucsdev/pcabot/develop/circt/llvm/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp:2500:7
<other frames from our own code follows>

[fzi-hielscher]

Could you point out where this crashes? I guess somewhere at the end of the Arcilator pipeline? Technically this pass is not specific to Arcilator and in general we consider i0 indices to be legal for single element arrays. So I'm wondering if there could be a more general fix, e.g., by turning them into scalars before the crash occurs.

[pscabot]

Could you point out where this crashes? I guess somewhere at the end of the Arcilator pipeline? Technically this pass is not specific to Arcilator and in general we consider i0 indices to be legal for single element arrays. So I'm wondering if there could be a more general fix, e.g., by turning them into scalars before the crash occurs.

I have updated the description with full crash information

[fzi-hielscher]

Thanks.

It generates the following llvm-mlir

%587 = llvm.mlir.constant(0 : i0) : i0
%588 = llvm.zext %587 : i0 to i1
%589 = llvm.getelementptr %68[0, %588] : (!llvm.ptr, i1) -> !llvm.ptr, !llvm.array<1 x i8>
%590 = llvm.load %589 : !llvm.ptr -> i8

The zext here should get folded to a i1 constant, see #8871. Is it possible you're missing a canonicalization pass before conversion to LLVM IR?

[pscabot]

Thanks.

It generates the following llvm-mlir

%587 = llvm.mlir.constant(0 : i0) : i0
%588 = llvm.zext %587 : i0 to i1
%589 = llvm.getelementptr %68[0, %588] : (!llvm.ptr, i1) -> !llvm.ptr, !llvm.array<1 x i8>
%590 = llvm.load %589 : !llvm.ptr -> i8

The zext here should get folded to a i1 constant, see #8871. Is it possible you're missing a canonicalization pass before conversion to LLVM IR?

We are definitely not doing this as we have our own tool and we are not using the ArcToLLVM conversion pass. We are directly using mlir::translateModuleToLLVMIR after the following passes:

1. ImportVerilog - Verilog to Core
2. Some internal passes
3. HWConvertBitcasts
4. ConvertToLLVM
5. ConvertHWToLLVM
6. ConvertToLLVMPass
7. ReconcileUnrealizedCasts

Now if this canonicalization is required then it should be part of ConvertHWToLLVM or similar. However, is there any reason not to fix it in HWConvertBitcasts? Do we expect to get i0 from other places?

[fzi-hielscher]

Okay, thanks for the context.

However, is there any reason not to fix it in HWConvertBitcasts? Do we expect to get i0 from other places?

Verilog does not support zero-width types, so in your scenario you are unlikely to encounter them. But other frontends do use them and they are a legal and supported type within the core dialects. Which is why I am against quietly introducing a rule that you should avoid them.

I'm okay with landing this as a temporary fix. But I think what we actually want is a dedicated "HWRemoveZeroWidthTypes" pass that actively strips them from the IR instead of relying on folders and canonicalizers.