[Mlir-commits] [mlir] 18439cf - [mlir][LLVMIR] Fix identified structs with same name

[Alexis Engelke]

Author: Alexis Engelke
Date: 2023-07-28T18:14:48+02:00
New Revision: 18439cfc14fe35884e75761819a63763d03fe46c

URL: https://github.com/llvm/llvm-project/commit/18439cfc14fe35884e75761819a63763d03fe46c
DIFF: https://github.com/llvm/llvm-project/commit/18439cfc14fe35884e75761819a63763d03fe46c.diff

LOG: [mlir][LLVMIR] Fix identified structs with same name

Different identified struct types may have the same name ("").
Previously, these were deduplicated based on their name, which caused
an assertion failure when nesting identified structs:

    %0 = type { %1 }
    %1 = type { i8 }
    declare void @fn(%0)

Reviewed By: gysit

Differential Revision: https://reviews.llvm.org/D156531

Added: 
    mlir/test/Target/LLVMIR/Import/global-struct.ll

Modified: 
    mlir/lib/Target/LLVMIR/TypeFromLLVM.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Target/LLVMIR/TypeFromLLVM.cpp b/mlir/lib/Target/LLVMIR/TypeFromLLVM.cpp
index 458e71953e6cf0..a4db958207756c 100644
--- a/mlir/lib/Target/LLVMIR/TypeFromLLVM.cpp
+++ b/mlir/lib/Target/LLVMIR/TypeFromLLVM.cpp
@@ -112,14 +112,13 @@ class TypeFromLLVMIRTranslatorImpl {
     if (type->isOpaque())
       return LLVM::LLVMStructType::getOpaque(type->getName(), &context);
 
-    LLVM::LLVMStructType translated =
-        LLVM::LLVMStructType::getIdentified(&context, type->getName());
-    knownTranslations.try_emplace(type, translated);
+    // With opaque pointers, types in LLVM can't be recursive anymore. Note that
+    // using getIdentified is not possible, as type names in LLVM are not
+    // guaranteed to be unique.
     translateTypes(type->subtypes(), subtypes);
-    LogicalResult bodySet = translated.setBody(subtypes, type->isPacked());
-    assert(succeeded(bodySet) &&
-           "could not set the body of an identified struct");
-    (void)bodySet;
+    LLVM::LLVMStructType translated = LLVM::LLVMStructType::getNewIdentified(
+        &context, type->getName(), subtypes, type->isPacked());
+    knownTranslations.try_emplace(type, translated);
     return translated;
   }
 

diff  --git a/mlir/test/Target/LLVMIR/Import/global-struct.ll b/mlir/test/Target/LLVMIR/Import/global-struct.ll
new file mode 100644
index 00000000000000..5e72b380537d66
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/Import/global-struct.ll
@@ -0,0 +1,8 @@
+; RUN: mlir-translate --import-llvm %s | FileCheck %s
+
+; Ensure both structs have 
diff erent names.
+; CHECK: llvm.func @fn(!llvm.struct<"[[NAME:[^"]*]]",
+; CHECK-NOT: struct<"[[NAME]]",
+%0 = type { %1 }
+%1 = type { i8 }
+declare void @fn(%0)