[mlir][ToLLVM] Fix the index bitwidth handling for the dynamic case of convert-to-llvm

[fabianmcg]

This patch changes the behavior of convert-to-llvm{dynamic=true} so that the nearest DataLayout is used to configure LowerToLLVMOptions and LLVMTypeConverter.

Example:

module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 16>>} {
  func.func private @test_16bit_index(%arg0: index) -> index
}
// mlir-opt --convert-to-llvm="dynamic=true"
module attributes {dlti.dl_spec = #dlti.dl_spec<index = 16 : i64>} {
  llvm.func @test_16bit_index(i16) -> i16 attributes {sym_visibility = "private"}
}

[Copilot]

Pull Request Overview

This patch fixes the handling of index bitwidth in the dynamic case of the convert-to-llvm pass by ensuring the nearest DataLayout is used to configure LowerToLLVMOptions and LLVMTypeConverter. This enables proper type conversion of index types based on the data layout specification.

• Updates the DynamicConvertToLLVM implementation to use DataLayout-aware LowerToLLVMOptions
• Adds test coverage for various index bitwidths (16, 32, 64) with dynamic conversion

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
mlir/lib/Conversion/ConvertToLLVM/ConvertToLLVMPass.cpp	Updated to extract DataLayout and create LowerToLLVMOptions with proper index bitwidth configuration
mlir/test/Conversion/FuncToLLVM/func-to-llvm-datalayout.mlir	Added comprehensive test cases verifying correct index type conversion for different bitwidths

[zero9178]

It looks to me as if the same logic is required in the static driver as well. Would it make sense to run the test with both "dynamic=true" and false?

Approving otherwise, feel free to defer to the future if youd like :)

[fabianmcg]

This fix doesn't work for the static case as the TypeConverter is created at pass initialization, which doesn't know on which op is being run on, so it's not possible to get the DataLayout to configure the converter.

[zero9178]

This fix doesn't work for the static case as the TypeConverter is created at pass initialization, which doesn't know on which op is being run on, so it's not possible to get the DataLayout to configure the converter.

Is it necessary in the static case for it be performed at pass initialization though?

[fabianmcg]

Is it necessary in the static case for it be performed at pass initialization though?

It was a performance constraint, that's also why the dynamic version was introduced. However, if I recall correctly @joker-eph we discussed the possibility to remove the static version, right?

[joker-eph]

Right!