[all-commits] [llvm/llvm-project] 43bc81: [mlir] fix LLVM type converter for structs (#73231)
ftynse via All-commits
all-commits at lists.llvm.org
Thu Nov 23 13:21:52 PST 2023
Branch: refs/heads/main
Home: https://github.com/llvm/llvm-project
Commit: 43bc81d7488a8fbd43b855f7e1100cfe110f90fc
https://github.com/llvm/llvm-project/commit/43bc81d7488a8fbd43b855f7e1100cfe110f90fc
Author: Oleksandr "Alex" Zinenko <zinenko at google.com>
Date: 2023-11-23 (Thu, 23 Nov 2023)
Changed paths:
M mlir/docs/TargetLLVMIR.md
M mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
A mlir/test/Conversion/LLVMCommon/types.mlir
Log Message:
-----------
[mlir] fix LLVM type converter for structs (#73231)
Existing implementation of the LLVM type converter for LLVM structs
containing incompatible types was attempting to change identifiers of
the struct in case of name clash post-conversion (all identified structs
have different names post-conversion since one cannot change the body of
the struct once initialized). Beyond a trivial error of not updating the
counter in renaming, this approach was broken for recursive structs that
can't be made aware of the renaming and would use the pre-existing
struct with clashing name instead.
For example, given
`!llvm.struct<"_Converted.foo", (struct<"_Converted.foo">, f32)>`
the following type
`!llvm.struct<"foo", (struct<"foo", index>)>`
would incorrectly convert to
```
!llvm.struct<"_Converted_1.foo",
(struct<"_Converted.foo",
(struct<"_Converted.foo">, f32)>)>
```
Remove this incorrect renaming and simply refuse to convert types if it
would lead to identifier clashes for structs with different bodies.
Document the expectation that such generated names are reserved and must
not be present in the input IR of the converter. If we ever actually
need to use handle such cases, this can be achieved by temporarily
renaming structs with reserved identifiers to an unreserved name and
back in a pre/post-processing pass that does _not_ use the type
conversion infra.
More information about the All-commits
mailing list