[MLIR][LLVMIR] Fix import of ConstantAggregateZero to use zero instead of undef

Author: frabert

mlir/lib/Target/LLVMIR/ModuleImport.cpp

@@ -1681,17 +1681,25 @@ FailureOr<Value> ModuleImport::convertConstant(llvm::Constant *constant) {
     return lookupValue(inst);
   }
 
+  // Convert zero-initialized aggregates to ZeroOp.
+  if (auto *aggregateZero = dyn_cast<llvm::ConstantAggregateZero>(constant)) {
+    Type type = convertType(aggregateZero->getType());
+    return builder.create<ZeroOp>(loc, type).getResult();
+  }
+
   // Convert aggregate constants.
   if (isa<llvm::ConstantAggregate>(constant) ||
       isa<llvm::ConstantAggregateZero>(constant)) {
     // Lookup the aggregate elements that have been converted before.
     SmallVector<Value> elementValues;
+    bool isAggregateZero = false;
     if (auto *constAgg = dyn_cast<llvm::ConstantAggregate>(constant)) {
       elementValues.reserve(constAgg->getNumOperands());
       for (llvm::Value *operand : constAgg->operands())
         elementValues.push_back(lookupValue(operand));
     }
     if (auto *constAgg = dyn_cast<llvm::ConstantAggregateZero>(constant)) {
+      isAggregateZero = true;
       unsigned numElements = constAgg->getElementCount().getFixedValue();
       elementValues.reserve(numElements);
       for (unsigned i = 0, e = numElements; i != e; ++i)
@@ -1700,12 +1708,18 @@ FailureOr<Value> ModuleImport::convertConstant(llvm::Constant *constant) {
     assert(llvm::count(elementValues, nullptr) == 0 &&
            "expected all elements have been converted before");
 
-    // Generate an UndefOp as root value and insert the aggregate elements.
+    // Generate a root value and insert the aggregate elements.
+    // For ConstantAggregateZero, use ZeroOp to preserve zero-initialization
+    // semantics. Otherwise use UndefOp as the root.
     Type rootType = convertType(constant->getType());
     bool isArrayOrStruct = isa<LLVMArrayType, LLVMStructType>(rootType);
     assert((isArrayOrStruct || LLVM::isCompatibleVectorType(rootType)) &&
            "unrecognized aggregate type");
-    Value root = builder.create<UndefOp>(loc, rootType);
+    Value root;
+    if (isAggregateZero)
+      root = builder.create<ZeroOp>(loc, rootType);
+    else
+      root = builder.create<UndefOp>(loc, rootType);
     for (const auto &it : llvm::enumerate(elementValues)) {
       if (isArrayOrStruct) {
         root = builder.create<InsertValueOp>(loc, root, it.value(), it.index());

mlir/test/Target/LLVMIR/Import/constant.ll

@@ -198,13 +198,8 @@ define i32 @function_address_after_def() {
 %nested_agg_type = type {%simple_agg_type, ptr}
 @nested_agg = global %nested_agg_type { %simple_agg_type{i32 1, i8 2, i16 3, i32 4}, ptr null }
 
-; CHECK-DAG:  %[[NULL:.+]] = llvm.mlir.zero : !llvm.ptr
-; CHECK-DAG:  %[[ROOT:.+]] = llvm.mlir.undef : vector<2x!llvm.ptr>
-; CHECK-DAG:  %[[P0:.+]] = llvm.mlir.constant(0 : i32) : i32
-; CHECK-DAG:  %[[CHAIN0:.+]] = llvm.insertelement %[[NULL]], %[[ROOT]][%[[P0]] : i32] : vector<2x!llvm.ptr>
-; CHECK-DAG:  %[[P1:.+]] = llvm.mlir.constant(1 : i32) : i32
-; CHECK-DAG:  %[[CHAIN1:.+]] = llvm.insertelement %[[NULL]], %[[CHAIN0]][%[[P1]] : i32] : vector<2x!llvm.ptr>
-; CHECK-DAG:  llvm.return %[[CHAIN1]] : vector<2x!llvm.ptr>
+; CHECK-DAG:  %[[VEC:.+]] = llvm.mlir.zero : vector<2x!llvm.ptr>
+; CHECK-DAG:  llvm.return %[[VEC]] : vector<2x!llvm.ptr>
 @vector_agg = global <2 x ptr> <ptr null, ptr null>
 
 ; // -----
@@ -274,3 +269,46 @@ define void @call_alias_func() {
 
 ; CHECK-LABEL: @call_alias_func()
 ; CHECK: llvm.dso_local_equivalent @alias_func : !llvm.ptr
+
+; // -----
+
+; Test that zeroinitializer for zero-element arrays is correctly translated
+; to llvm.mlir.zero instead of llvm.mlir.undef. This is a regression test for
+; a bug where empty aggregate constants were incorrectly converted to undef.
+
+@global_zero_array = global [0 x ptr] zeroinitializer
+
+; CHECK: llvm.mlir.global external @global_zero_array() {addr_space = 0 : i32} : !llvm.array<0 x ptr> {
+; CHECK:   %[[ZERO:.+]] = llvm.mlir.zero : !llvm.array<0 x ptr>
+; CHECK:   llvm.return %[[ZERO]] : !llvm.array<0 x ptr>
+; CHECK: }
+
+; CHECK-LABEL: @load_zero_array
+define [0 x ptr] @load_zero_array() {
+  ; CHECK: %[[ADDR:.+]] = llvm.mlir.addressof @global_zero_array : !llvm.ptr
+  ; CHECK: %[[VAL:.+]] = llvm.load %[[ADDR]] {{.*}}: !llvm.ptr -> !llvm.array<0 x ptr>
+  ; CHECK: llvm.return %[[VAL]] : !llvm.array<0 x ptr>
+  %val = load [0 x ptr], ptr @global_zero_array
+  ret [0 x ptr] %val
+}
+
+; // -----
+
+; Test that zeroinitializer for zero-element structs is correctly handled
+
+@global_zero_struct = global {} zeroinitializer
+
+; CHECK: llvm.mlir.global external @global_zero_struct() {addr_space = 0 : i32} : !llvm.struct<()> {
+; CHECK:   %[[ZERO:.+]] = llvm.mlir.zero : !llvm.struct<()>
+; CHECK:   llvm.return %[[ZERO]] : !llvm.struct<()>
+; CHECK: }
+
+; // -----
+
+; Test that zeroinitializer for arrays with elements still works correctly.
+; Note that arrays with primitive types that can be represented as dense
+; attributes may use the attribute form directly.
+
+@global_array_with_elements = global [3 x i32] zeroinitializer
+
+; CHECK: llvm.mlir.global external @global_array_with_elements({{.*}}) {addr_space = 0 : i32} : !llvm.array<3 x i32>

mlir/test/Target/LLVMIR/Import/exception.ll

@@ -123,9 +123,7 @@ define void @landingpad_dominance() personality ptr @__gxx_personality_v0 {
 entry:
   ; CHECK:    %[[null:.*]] = llvm.mlir.zero : !llvm.ptr
   ; CHECK:    %[[c1:.*]] = llvm.mlir.constant(0 : i32) : i32
-  ; CHECK:    %[[undef:.*]] = llvm.mlir.undef : !llvm.struct<(ptr, i32)>
-  ; CHECK:    %[[tmpstruct:.*]] = llvm.insertvalue %[[null]], %[[undef]][0] : !llvm.struct<(ptr, i32)>
-  ; CHECK:    %[[struct:.*]] = llvm.insertvalue %[[c1]], %[[tmpstruct]][1] : !llvm.struct<(ptr, i32)>
+  ; CHECK:    %[[struct:.*]] = llvm.mlir.zero : !llvm.struct<(ptr, i32)>
   ; CHECK:    llvm.call @f0(%[[null]]) : (!llvm.ptr) -> ()
   call void @f0(ptr null)
   ; CHECK:    llvm.call @f1(%[[c1]]) : (i32) -> ()
@@ -190,3 +188,59 @@ bb3:
 !6 = !DILocation(line: 2, column: 2, scope: !3)
 !7 = !DILocation(line: 7, column: 4, scope: !4, inlinedAt: !6)
 !8 = !DILocalVariable(scope: !4, name: "size")
+
+; // -----
+
+declare i32 @__gxx_personality_v0(...)
+declare void @foo(ptr)
+
+; Test that landingpad filter clauses with zeroinitializer are correctly
+; translated to llvm.mlir.zero instead of llvm.mlir.undef.
+; This is a regression test for a bug where zero-element arrays used in
+; filter clauses were incorrectly converted to undef.
+
+; CHECK-LABEL: @landingpad_zero_filter
+define void @landingpad_zero_filter() personality ptr @__gxx_personality_v0 {
+entry:
+  ; CHECK: %[[ZERO:.+]] = llvm.mlir.zero : !llvm.array<0 x ptr>
+  invoke void @foo(ptr null) to label %normal unwind label %lpad
+
+normal:
+  ret void
+
+lpad:
+  ; CHECK: %{{[0-9]+}} = llvm.landingpad cleanup (filter %[[ZERO]] : !llvm.array<0 x ptr>) : !llvm.struct<(ptr, i32)>
+  %0 = landingpad { ptr, i32 }
+          cleanup
+          filter [0 x ptr] zeroinitializer
+  ret void
+}
+
+; // -----
+
+declare i32 @__gxx_personality_v0(...)
+declare void @foo(ptr)
+
+; Test that landingpad with multiple filter clauses of different zero-element
+; array types are correctly handled. Note that zero-element arrays of primitive
+; types like i32 may be converted to dense attributes, while ptr arrays use
+; llvm.mlir.zero.
+
+; CHECK-LABEL: @landingpad_mixed_filters
+define void @landingpad_mixed_filters() personality ptr @__gxx_personality_v0 {
+entry:
+  ; CHECK: %[[ZERO1:.+]] = llvm.mlir.zero : !llvm.array<0 x ptr>
+  ; CHECK: %[[ZERO2:.+]] = llvm.mlir.{{(zero|constant)}}{{.*}}: !llvm.array<0 x i32>
+  invoke void @foo(ptr null) to label %normal unwind label %lpad
+
+normal:
+  ret void
+
+lpad:
+  ; CHECK: %{{[0-9]+}} = llvm.landingpad cleanup (filter %[[ZERO1]] : !llvm.array<0 x ptr>) (filter %[[ZERO2]] : !llvm.array<0 x i32>) : !llvm.struct<(ptr, i32)>
+  %0 = landingpad { ptr, i32 }
+          cleanup
+          filter [0 x ptr] zeroinitializer
+          filter [0 x i32] zeroinitializer
+  ret void
+}