[Mlir-commits] [mlir] 3674cad - [mlir][llvm] Fix bug in the LLVM IR constant import.

[Tobias Gysi]

Author: Tobias Gysi
Date: 2022-12-14T10:27:12+01:00
New Revision: 3674cadf4d6a3390875bd0173470f3a55870edcd

URL: https://github.com/llvm/llvm-project/commit/3674cadf4d6a3390875bd0173470f3a55870edcd
DIFF: https://github.com/llvm/llvm-project/commit/3674cadf4d6a3390875bd0173470f3a55870edcd.diff

LOG: [mlir][llvm] Fix bug in the LLVM IR constant import.

The recently introduced iterative constant import
(https://reviews.llvm.org/D137559) fails for programs that
subsequently import constant expressions with duplicate
subexpressions. The reason is a broken duplicate check
in getConstantsToConvert. The revision fixes the bug and
adds a test case that imports two constant expressions
with duplicates.

Reviewed By: ftynse

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

Added: 
    

Modified: 
    mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
    mlir/test/Target/LLVMIR/Import/constant.ll

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
index c8f9dab94ef9f..ec9bde22e58f6 100644
--- a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -688,7 +688,7 @@ Importer::getConstantsToConvert(llvm::Constant *constant) {
   while (!workList.empty()) {
     llvm::Constant *current = workList.pop_back_val();
     // Skip constants that have been converted before and store all other ones.
-    if (valueMapping.count(constant))
+    if (valueMapping.count(current))
       continue;
     orderedList.push_back(current);
     // Add the current constant's dependencies to the work list. Only add

diff  --git a/mlir/test/Target/LLVMIR/Import/constant.ll b/mlir/test/Target/LLVMIR/Import/constant.ll
index e1889d7691b2a..98bd8e8e0e1b3 100644
--- a/mlir/test/Target/LLVMIR/Import/constant.ll
+++ b/mlir/test/Target/LLVMIR/Import/constant.ll
@@ -208,3 +208,21 @@ define i32 @function_address_after_def() {
 ; CHECK:  %[[CHAIN1:.+]] = llvm.insertelement %[[NULL]], %[[CHAIN0]][%[[P1]] : i32] : !llvm.vec<2 x ptr<struct<"simple_agg_type", (i32, i8, i16, i32)>>>
 ; CHECK:  llvm.return %[[CHAIN1]] : !llvm.vec<2 x ptr<struct<"simple_agg_type", (i32, i8, i16, i32)>>>
 @vector_agg = global <2 x %simple_agg_type*> <%simple_agg_type* null, %simple_agg_type* null>
+
+; // -----
+
+; Verfiy the import of subsequent constant expressions with duplicates.
+
+ at global = external global i32, align 8
+
+; CHECK-LABEL: @const_exprs_with_duplicate
+define i64 @const_exprs_with_duplicate() {
+  ; CHECK: %[[ADDR:.+]] = llvm.mlir.addressof @global : !llvm.ptr<i32>
+  ; CHECK: llvm.getelementptr %[[ADDR]][%{{.*}}] : (!llvm.ptr<i32>, i32) -> !llvm.ptr<i32>
+  %1 = add i64 1, ptrtoint (i32* getelementptr (i32, i32* @global, i32 7) to i64)
+
+  ; Verify the address value is reused.
+  ; CHECK: llvm.getelementptr %[[ADDR]][%{{.*}}] : (!llvm.ptr<i32>, i32) -> !llvm.ptr<i32>
+  %2 = add i64 %1, ptrtoint (i32* getelementptr (i32, i32* @global, i32 42) to i64)
+  ret i64 %2
+}