Skip to content

[mlir][LLVM] Switch undef for poison for uninitialized values #125629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 6, 2025

Conversation

krzysz00
Copy link
Contributor

@krzysz00 krzysz00 commented Feb 4, 2025

LLVM itself is generally moving away from using undef and towards using poison, to the point of having a lint that caches new uses of undef in tests.

In order to not trip the lint on new patterns and to conform to the evolution of LLVM

  • Rename valious ::undef() methods on StructBuilder subclasses to ::poison()
  • Audit the uses of UndefOp in the MLIR libraries and replace almost all of them with PoisonOp

The remaining uses of undef are initializing uninitialized memrefs, explicit conversions to undef from SPIR-V, and a few cases in AMDGPUToROCDL where usage like

%v = insertelement <M x iN> undef, iN %v, i32 0
%arg = bitcast <M x iN> %v to i(M * N)

is used to handle "i32" arguments that are are really packed vectors of smaller types that won't always be fully initialized.

LLVM itself is generally moving away from using `undef` and towards
using `poison`, to the point of having a lint that caches new uses of
`undef` in tests.

In order to not trip the lint on new patterns and to conform to the
evolution of LLVM
- Rename valious ::undef() methods on StructBuilder subclasses to
::poison()
- Audit the uses of UndefOp in the MLIR libraries and replace almost
all of them with PoisonOp

The remaining uses of `undef` are initializing `uninitialized`
memrefs, explicit conversions to undef from SPIR-V, and a few cases in
AMDGPUToROCDL where usage like

    %v = insertelement <M x iN> undef, iN %v, i32 0
    %arg = bitcast <M x iN> %v to i(M * N)

is used to handle "i32" arguments that are are really packed vectors
of smaller types that won't always be fully initialized.
@llvmbot
Copy link
Member

llvmbot commented Feb 4, 2025

@llvm/pr-subscribers-mlir-sparse
@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-mlir-gpu

@llvm/pr-subscribers-mlir-spirv

Author: Krzysztof Drewniak (krzysz00)

Changes

LLVM itself is generally moving away from using undef and towards using poison, to the point of having a lint that caches new uses of undef in tests.

In order to not trip the lint on new patterns and to conform to the evolution of LLVM

  • Rename valious ::undef() methods on StructBuilder subclasses to ::poison()
  • Audit the uses of UndefOp in the MLIR libraries and replace almost all of them with PoisonOp

The remaining uses of undef are initializing uninitialized memrefs, explicit conversions to undef from SPIR-V, and a few cases in AMDGPUToROCDL where usage like

%v = insertelement &lt;M x iN&gt; undef, iN %v, i32 0
%arg = bitcast &lt;M x iN&gt; %v to i(M * N)

is used to handle "i32" arguments that are are really packed vectors of smaller types that won't always be fully initialized.


Patch is 232.26 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/125629.diff

39 Files Affected:

  • (modified) mlir/include/mlir/Conversion/ComplexToLLVM/ComplexToLLVM.h (+2-2)
  • (modified) mlir/include/mlir/Conversion/LLVMCommon/MemRefBuilder.h (+5-5)
  • (modified) mlir/include/mlir/Conversion/LLVMCommon/StructBuilder.h (+3-3)
  • (modified) mlir/lib/Conversion/ArmSMEToLLVM/ArmSMEToLLVM.cpp (+1-1)
  • (modified) mlir/lib/Conversion/ComplexToLLVM/ComplexToLLVM.cpp (+9-8)
  • (modified) mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp (+2-2)
  • (modified) mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp (+2-2)
  • (modified) mlir/lib/Conversion/GPUToNVVM/WmmaOpsToNvvm.cpp (+3-3)
  • (modified) mlir/lib/Conversion/LLVMCommon/MemRefBuilder.cpp (+10-10)
  • (modified) mlir/lib/Conversion/LLVMCommon/Pattern.cpp (+2-2)
  • (modified) mlir/lib/Conversion/LLVMCommon/VectorPattern.cpp (+1-1)
  • (modified) mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp (+9-9)
  • (modified) mlir/lib/Conversion/NVGPUToNVVM/NVGPUToNVVM.cpp (+6-6)
  • (modified) mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp (+3-3)
  • (modified) mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp (+11-10)
  • (modified) mlir/lib/Dialect/SparseTensor/Transforms/SparseStorageSpecifierToLLVM.cpp (+1-1)
  • (modified) mlir/test/Conversion/ArithToLLVM/convert-nd-vector-to-llvmir.mlir (+14-14)
  • (modified) mlir/test/Conversion/ComplexToLLVM/convert-to-llvm.mlir (+9-9)
  • (modified) mlir/test/Conversion/ComplexToLLVM/full-conversion.mlir (+2-2)
  • (modified) mlir/test/Conversion/FuncToLLVM/calling-convention.mlir (+16-16)
  • (modified) mlir/test/Conversion/FuncToLLVM/func-memref-return.mlir (+3-3)
  • (modified) mlir/test/Conversion/FuncToLLVM/func-to-llvm.mlir (+4-4)
  • (modified) mlir/test/Conversion/GPUCommon/memory-attrbution.mlir (+6-6)
  • (modified) mlir/test/Conversion/GPUToLLVMSPV/gpu-to-llvm-spv.mlir (+9-9)
  • (modified) mlir/test/Conversion/GPUToNVVM/gpu-to-nvvm.mlir (+3-3)
  • (modified) mlir/test/Conversion/GPUToNVVM/wmma-ops-to-nvvm.mlir (+4-4)
  • (modified) mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir (+1-1)
  • (modified) mlir/test/Conversion/MemRefToLLVM/convert-dynamic-memref-ops.mlir (+12-12)
  • (modified) mlir/test/Conversion/MemRefToLLVM/convert-static-memref-ops.mlir (+7-7)
  • (modified) mlir/test/Conversion/MemRefToLLVM/expand-then-convert-to-llvm.mlir (+18-18)
  • (modified) mlir/test/Conversion/MemRefToLLVM/memref-to-llvm.mlir (+18-18)
  • (modified) mlir/test/Conversion/NVGPUToNVVM/nvgpu-to-nvvm.mlir (+225-225)
  • (modified) mlir/test/Conversion/SPIRVToLLVM/bitwise-ops-to-llvm.mlir (+6-6)
  • (modified) mlir/test/Conversion/SPIRVToLLVM/misc-ops-to-llvm.mlir (+3-3)
  • (modified) mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir (+18-18)
  • (modified) mlir/test/Dialect/GPU/dynamic-shared-memory.mlir (+3-3)
  • (modified) mlir/test/Dialect/LLVM/lower-to-llvm-e2e-with-target-tag.mlir (+2-2)
  • (modified) mlir/test/Dialect/LLVM/lower-to-llvm-e2e-with-top-level-named-sequence.mlir (+2-2)
  • (modified) mlir/test/Dialect/SparseTensor/specifier_to_llvm.mlir (+1-1)
diff --git a/mlir/include/mlir/Conversion/ComplexToLLVM/ComplexToLLVM.h b/mlir/include/mlir/Conversion/ComplexToLLVM/ComplexToLLVM.h
index 65d6f165c7448d..8266442cf5db81 100644
--- a/mlir/include/mlir/Conversion/ComplexToLLVM/ComplexToLLVM.h
+++ b/mlir/include/mlir/Conversion/ComplexToLLVM/ComplexToLLVM.h
@@ -24,8 +24,8 @@ class ComplexStructBuilder : public StructBuilder {
   /// Construct a helper for the given complex number value.
   using StructBuilder::StructBuilder;
   /// Build IR creating an `undef` value of the complex number type.
-  static ComplexStructBuilder undef(OpBuilder &builder, Location loc,
-                                    Type type);
+  static ComplexStructBuilder poison(OpBuilder &builder, Location loc,
+                                     Type type);
 
   // Build IR extracting the real value from the complex number struct.
   Value real(OpBuilder &builder, Location loc);
diff --git a/mlir/include/mlir/Conversion/LLVMCommon/MemRefBuilder.h b/mlir/include/mlir/Conversion/LLVMCommon/MemRefBuilder.h
index c39e48b5de752e..d5055f023cdc81 100644
--- a/mlir/include/mlir/Conversion/LLVMCommon/MemRefBuilder.h
+++ b/mlir/include/mlir/Conversion/LLVMCommon/MemRefBuilder.h
@@ -34,9 +34,9 @@ class MemRefDescriptor : public StructBuilder {
 public:
   /// Construct a helper for the given descriptor value.
   explicit MemRefDescriptor(Value descriptor);
-  /// Builds IR creating an `undef` value of the descriptor type.
-  static MemRefDescriptor undef(OpBuilder &builder, Location loc,
-                                Type descriptorType);
+  /// Builds IR creating a `poison` value of the descriptor type.
+  static MemRefDescriptor poison(OpBuilder &builder, Location loc,
+                                 Type descriptorType);
   /// Builds IR creating a MemRef descriptor that represents `type` and
   /// populates it with static shape and stride information extracted from the
   /// type.
@@ -160,8 +160,8 @@ class UnrankedMemRefDescriptor : public StructBuilder {
   /// Construct a helper for the given descriptor value.
   explicit UnrankedMemRefDescriptor(Value descriptor);
   /// Builds IR creating an `undef` value of the descriptor type.
-  static UnrankedMemRefDescriptor undef(OpBuilder &builder, Location loc,
-                                        Type descriptorType);
+  static UnrankedMemRefDescriptor poison(OpBuilder &builder, Location loc,
+                                         Type descriptorType);
 
   /// Builds IR extracting the rank from the descriptor
   Value rank(OpBuilder &builder, Location loc) const;
diff --git a/mlir/include/mlir/Conversion/LLVMCommon/StructBuilder.h b/mlir/include/mlir/Conversion/LLVMCommon/StructBuilder.h
index 1a5b97eb928303..b6ff1d13abf1da 100644
--- a/mlir/include/mlir/Conversion/LLVMCommon/StructBuilder.h
+++ b/mlir/include/mlir/Conversion/LLVMCommon/StructBuilder.h
@@ -27,9 +27,9 @@ class StructBuilder {
 public:
   /// Construct a helper for the given value.
   explicit StructBuilder(Value v);
-  /// Builds IR creating an `undef` value of the descriptor type.
-  static StructBuilder undef(OpBuilder &builder, Location loc,
-                             Type descriptorType);
+  /// Builds IR creating a `poison` value of the descriptor type.
+  static StructBuilder poison(OpBuilder &builder, Location loc,
+                              Type descriptorType);
 
   /*implicit*/ operator Value() { return value; }
 
diff --git a/mlir/lib/Conversion/ArmSMEToLLVM/ArmSMEToLLVM.cpp b/mlir/lib/Conversion/ArmSMEToLLVM/ArmSMEToLLVM.cpp
index 40a3489f7a4d7b..417555792b44fa 100644
--- a/mlir/lib/Conversion/ArmSMEToLLVM/ArmSMEToLLVM.cpp
+++ b/mlir/lib/Conversion/ArmSMEToLLVM/ArmSMEToLLVM.cpp
@@ -317,7 +317,7 @@ struct ConvertArmSMESpillsAndFillsToLLVM : public ConvertToLLVMPattern {
     auto allTruePredicate = rewriter.create<arith::ConstantOp>(
         loc, DenseElementsAttr::get(predicateType, true));
     // Create padding vector (never used due to all-true predicate).
-    auto padVector = rewriter.create<LLVM::UndefOp>(loc, sliceType);
+    auto padVector = rewriter.create<LLVM::PoisonOp>(loc, sliceType);
     // Get a pointer to the current slice.
     auto slicePtr =
         getInMemoryTileSlicePtr(rewriter, loc, tileAlloca, sliceIndex);
diff --git a/mlir/lib/Conversion/ComplexToLLVM/ComplexToLLVM.cpp b/mlir/lib/Conversion/ComplexToLLVM/ComplexToLLVM.cpp
index fcc18e6eee9980..ad86fe362076b2 100644
--- a/mlir/lib/Conversion/ComplexToLLVM/ComplexToLLVM.cpp
+++ b/mlir/lib/Conversion/ComplexToLLVM/ComplexToLLVM.cpp
@@ -33,9 +33,9 @@ using namespace mlir::arith;
 static constexpr unsigned kRealPosInComplexNumberStruct = 0;
 static constexpr unsigned kImaginaryPosInComplexNumberStruct = 1;
 
-ComplexStructBuilder ComplexStructBuilder::undef(OpBuilder &builder,
-                                                 Location loc, Type type) {
-  Value val = builder.create<LLVM::UndefOp>(loc, type);
+ComplexStructBuilder ComplexStructBuilder::poison(OpBuilder &builder,
+                                                  Location loc, Type type) {
+  Value val = builder.create<LLVM::PoisonOp>(loc, type);
   return ComplexStructBuilder(val);
 }
 
@@ -109,7 +109,8 @@ struct CreateOpConversion : public ConvertOpToLLVMPattern<complex::CreateOp> {
     // Pack real and imaginary part in a complex number struct.
     auto loc = complexOp.getLoc();
     auto structType = typeConverter->convertType(complexOp.getType());
-    auto complexStruct = ComplexStructBuilder::undef(rewriter, loc, structType);
+    auto complexStruct =
+        ComplexStructBuilder::poison(rewriter, loc, structType);
     complexStruct.setReal(rewriter, loc, adaptor.getReal());
     complexStruct.setImaginary(rewriter, loc, adaptor.getImaginary());
 
@@ -183,7 +184,7 @@ struct AddOpConversion : public ConvertOpToLLVMPattern<complex::AddOp> {
 
     // Initialize complex number struct for result.
     auto structType = typeConverter->convertType(op.getType());
-    auto result = ComplexStructBuilder::undef(rewriter, loc, structType);
+    auto result = ComplexStructBuilder::poison(rewriter, loc, structType);
 
     // Emit IR to add complex numbers.
     arith::FastMathFlagsAttr complexFMFAttr = op.getFastMathFlagsAttr();
@@ -214,7 +215,7 @@ struct DivOpConversion : public ConvertOpToLLVMPattern<complex::DivOp> {
 
     // Initialize complex number struct for result.
     auto structType = typeConverter->convertType(op.getType());
-    auto result = ComplexStructBuilder::undef(rewriter, loc, structType);
+    auto result = ComplexStructBuilder::poison(rewriter, loc, structType);
 
     // Emit IR to add complex numbers.
     arith::FastMathFlagsAttr complexFMFAttr = op.getFastMathFlagsAttr();
@@ -262,7 +263,7 @@ struct MulOpConversion : public ConvertOpToLLVMPattern<complex::MulOp> {
 
     // Initialize complex number struct for result.
     auto structType = typeConverter->convertType(op.getType());
-    auto result = ComplexStructBuilder::undef(rewriter, loc, structType);
+    auto result = ComplexStructBuilder::poison(rewriter, loc, structType);
 
     // Emit IR to add complex numbers.
     arith::FastMathFlagsAttr complexFMFAttr = op.getFastMathFlagsAttr();
@@ -302,7 +303,7 @@ struct SubOpConversion : public ConvertOpToLLVMPattern<complex::SubOp> {
 
     // Initialize complex number struct for result.
     auto structType = typeConverter->convertType(op.getType());
-    auto result = ComplexStructBuilder::undef(rewriter, loc, structType);
+    auto result = ComplexStructBuilder::poison(rewriter, loc, structType);
 
     // Emit IR to substract complex numbers.
     arith::FastMathFlagsAttr complexFMFAttr = op.getFastMathFlagsAttr();
diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index 790e18d2fccebe..55f0a9ac3bbb23 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -660,7 +660,7 @@ struct UnrealizedConversionCastOpLowering
 // `ReturnOp` interacts with the function signature and must have as many
 // operands as the function has return values.  Because in LLVM IR, functions
 // can only return 0 or 1 value, we pack multiple values into a structure type.
-// Emit `UndefOp` followed by `InsertValueOp`s to create such structure if
+// Emit `PoisonOp` followed by `InsertValueOp`s to create such structure if
 // necessary before returning it
 struct ReturnOpLowering : public ConvertOpToLLVMPattern<func::ReturnOp> {
   using ConvertOpToLLVMPattern<func::ReturnOp>::ConvertOpToLLVMPattern;
@@ -714,7 +714,7 @@ struct ReturnOpLowering : public ConvertOpToLLVMPattern<func::ReturnOp> {
       return rewriter.notifyMatchFailure(op, "could not convert result types");
     }
 
-    Value packed = rewriter.create<LLVM::UndefOp>(loc, packedType);
+    Value packed = rewriter.create<LLVM::PoisonOp>(loc, packedType);
     for (auto [idx, operand] : llvm::enumerate(updatedOperands)) {
       packed = rewriter.create<LLVM::InsertValueOp>(loc, packed, operand, idx);
     }
diff --git a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
index 544fc57949e24d..cfa434699cdef5 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
@@ -603,7 +603,7 @@ LogicalResult impl::scalarizeVectorOp(Operation *op, ValueRange operands,
     return rewriter.notifyMatchFailure(op, "expected vector result");
 
   Location loc = op->getLoc();
-  Value result = rewriter.create<LLVM::UndefOp>(loc, vectorType);
+  Value result = rewriter.create<LLVM::PoisonOp>(loc, vectorType);
   Type indexType = converter.convertType(rewriter.getIndexType());
   StringAttr name = op->getName().getIdentifier();
   Type elementType = vectorType.getElementType();
@@ -771,7 +771,7 @@ LogicalResult GPUReturnOpLowering::matchAndRewrite(
     return rewriter.notifyMatchFailure(op, "could not convert result types");
   }
 
-  Value packed = rewriter.create<LLVM::UndefOp>(loc, packedType);
+  Value packed = rewriter.create<LLVM::PoisonOp>(loc, packedType);
   for (auto [idx, operand] : llvm::enumerate(updatedOperands)) {
     packed = rewriter.create<LLVM::InsertValueOp>(loc, packed, operand, idx);
   }
diff --git a/mlir/lib/Conversion/GPUToNVVM/WmmaOpsToNvvm.cpp b/mlir/lib/Conversion/GPUToNVVM/WmmaOpsToNvvm.cpp
index 2b040fddac7486..4b50b9187b25bd 100644
--- a/mlir/lib/Conversion/GPUToNVVM/WmmaOpsToNvvm.cpp
+++ b/mlir/lib/Conversion/GPUToNVVM/WmmaOpsToNvvm.cpp
@@ -279,7 +279,7 @@ struct WmmaConstantOpToNVVMLowering
         cast<gpu::MMAMatrixType>(subgroupMmaConstantOp.getType()));
     // If the element type is a vector create a vector from the operand.
     if (auto vecType = dyn_cast<VectorType>(type.getBody()[0])) {
-      Value vecCst = rewriter.create<LLVM::UndefOp>(loc, vecType);
+      Value vecCst = rewriter.create<LLVM::PoisonOp>(loc, vecType);
       for (int64_t vecEl = 0; vecEl < vecType.getNumElements(); vecEl++) {
         Value idx = rewriter.create<LLVM::ConstantOp>(
             loc, rewriter.getI32Type(), vecEl);
@@ -288,7 +288,7 @@ struct WmmaConstantOpToNVVMLowering
       }
       cst = vecCst;
     }
-    Value matrixStruct = rewriter.create<LLVM::UndefOp>(loc, type);
+    Value matrixStruct = rewriter.create<LLVM::PoisonOp>(loc, type);
     for (size_t i : llvm::seq(size_t(0), type.getBody().size())) {
       matrixStruct =
           rewriter.create<LLVM::InsertValueOp>(loc, matrixStruct, cst, i);
@@ -355,7 +355,7 @@ struct WmmaElementwiseOpToNVVMLowering
     size_t numOperands = adaptor.getOperands().size();
     LLVM::LLVMStructType destType = convertMMAToLLVMType(
         cast<gpu::MMAMatrixType>(subgroupMmaElementwiseOp.getType()));
-    Value matrixStruct = rewriter.create<LLVM::UndefOp>(loc, destType);
+    Value matrixStruct = rewriter.create<LLVM::PoisonOp>(loc, destType);
     for (size_t i = 0, e = destType.getBody().size(); i < e; ++i) {
       SmallVector<Value> extractedOperands;
       for (size_t opIdx = 0; opIdx < numOperands; opIdx++) {
diff --git a/mlir/lib/Conversion/LLVMCommon/MemRefBuilder.cpp b/mlir/lib/Conversion/LLVMCommon/MemRefBuilder.cpp
index 3d4b65c7be8f3f..86d66438203766 100644
--- a/mlir/lib/Conversion/LLVMCommon/MemRefBuilder.cpp
+++ b/mlir/lib/Conversion/LLVMCommon/MemRefBuilder.cpp
@@ -29,10 +29,10 @@ MemRefDescriptor::MemRefDescriptor(Value descriptor)
 }
 
 /// Builds IR creating an `undef` value of the descriptor type.
-MemRefDescriptor MemRefDescriptor::undef(OpBuilder &builder, Location loc,
-                                         Type descriptorType) {
+MemRefDescriptor MemRefDescriptor::poison(OpBuilder &builder, Location loc,
+                                          Type descriptorType) {
 
-  Value descriptor = builder.create<LLVM::UndefOp>(loc, descriptorType);
+  Value descriptor = builder.create<LLVM::PoisonOp>(loc, descriptorType);
   return MemRefDescriptor(descriptor);
 }
 
@@ -60,7 +60,7 @@ MemRefDescriptor MemRefDescriptor::fromStaticShape(
   auto convertedType = typeConverter.convertType(type);
   assert(convertedType && "unexpected failure in memref type conversion");
 
-  auto descr = MemRefDescriptor::undef(builder, loc, convertedType);
+  auto descr = MemRefDescriptor::poison(builder, loc, convertedType);
   descr.setAllocatedPtr(builder, loc, memory);
   descr.setAlignedPtr(builder, loc, alignedMemory);
   descr.setConstantOffset(builder, loc, offset);
@@ -224,7 +224,7 @@ Value MemRefDescriptor::pack(OpBuilder &builder, Location loc,
                              const LLVMTypeConverter &converter,
                              MemRefType type, ValueRange values) {
   Type llvmType = converter.convertType(type);
-  auto d = MemRefDescriptor::undef(builder, loc, llvmType);
+  auto d = MemRefDescriptor::poison(builder, loc, llvmType);
 
   d.setAllocatedPtr(builder, loc, values[kAllocatedPtrPosInMemRefDescriptor]);
   d.setAlignedPtr(builder, loc, values[kAlignedPtrPosInMemRefDescriptor]);
@@ -300,10 +300,10 @@ UnrankedMemRefDescriptor::UnrankedMemRefDescriptor(Value descriptor)
     : StructBuilder(descriptor) {}
 
 /// Builds IR creating an `undef` value of the descriptor type.
-UnrankedMemRefDescriptor UnrankedMemRefDescriptor::undef(OpBuilder &builder,
-                                                         Location loc,
-                                                         Type descriptorType) {
-  Value descriptor = builder.create<LLVM::UndefOp>(loc, descriptorType);
+UnrankedMemRefDescriptor UnrankedMemRefDescriptor::poison(OpBuilder &builder,
+                                                          Location loc,
+                                                          Type descriptorType) {
+  Value descriptor = builder.create<LLVM::PoisonOp>(loc, descriptorType);
   return UnrankedMemRefDescriptor(descriptor);
 }
 Value UnrankedMemRefDescriptor::rank(OpBuilder &builder, Location loc) const {
@@ -331,7 +331,7 @@ Value UnrankedMemRefDescriptor::pack(OpBuilder &builder, Location loc,
                                      UnrankedMemRefType type,
                                      ValueRange values) {
   Type llvmType = converter.convertType(type);
-  auto d = UnrankedMemRefDescriptor::undef(builder, loc, llvmType);
+  auto d = UnrankedMemRefDescriptor::poison(builder, loc, llvmType);
 
   d.setRank(builder, loc, values[kRankInUnrankedMemRefDescriptor]);
   d.setMemRefDescPtr(builder, loc, values[kPtrInUnrankedMemRefDescriptor]);
diff --git a/mlir/lib/Conversion/LLVMCommon/Pattern.cpp b/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
index 840bd3df61a063..71b68619cc7938 100644
--- a/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
+++ b/mlir/lib/Conversion/LLVMCommon/Pattern.cpp
@@ -218,7 +218,7 @@ MemRefDescriptor ConvertToLLVMPattern::createMemRefDescriptor(
     ArrayRef<Value> sizes, ArrayRef<Value> strides,
     ConversionPatternRewriter &rewriter) const {
   auto structType = typeConverter->convertType(memRefType);
-  auto memRefDescriptor = MemRefDescriptor::undef(rewriter, loc, structType);
+  auto memRefDescriptor = MemRefDescriptor::poison(rewriter, loc, structType);
 
   // Field 1: Allocated pointer, used for malloc/free.
   memRefDescriptor.setAllocatedPtr(rewriter, loc, allocatedPtr);
@@ -319,7 +319,7 @@ LogicalResult ConvertToLLVMPattern::copyUnrankedDescriptors(
     if (!descriptorType)
       return failure();
     auto updatedDesc =
-        UnrankedMemRefDescriptor::undef(builder, loc, descriptorType);
+        UnrankedMemRefDescriptor::poison(builder, loc, descriptorType);
     Value rank = desc.rank(builder, loc);
     updatedDesc.setRank(builder, loc, rank);
     updatedDesc.setMemRefDescPtr(builder, loc, memory);
diff --git a/mlir/lib/Conversion/LLVMCommon/VectorPattern.cpp b/mlir/lib/Conversion/LLVMCommon/VectorPattern.cpp
index 626135c10a3e96..bf3f31729c3da0 100644
--- a/mlir/lib/Conversion/LLVMCommon/VectorPattern.cpp
+++ b/mlir/lib/Conversion/LLVMCommon/VectorPattern.cpp
@@ -87,7 +87,7 @@ LogicalResult LLVM::detail::handleMultidimensionalVectors(
   auto result1DVectorTy = resultTypeInfo.llvm1DVectorTy;
   auto resultNDVectoryTy = resultTypeInfo.llvmNDVectorTy;
   auto loc = op->getLoc();
-  Value desc = rewriter.create<LLVM::UndefOp>(loc, resultNDVectoryTy);
+  Value desc = rewriter.create<LLVM::PoisonOp>(loc, resultNDVectoryTy);
   nDVectorIterate(resultTypeInfo, rewriter, [&](ArrayRef<int64_t> position) {
     // For this unrolled `position` corresponding to the `linearIndex`^th
     // element, extract operand vectors
diff --git a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
index af1dba4587dc1f..3646416def8102 100644
--- a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
+++ b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
@@ -714,10 +714,10 @@ struct MemRefCastOpLowering : public ConvertOpToLLVMPattern<memref::CastOp> {
       // rank = ConstantOp srcRank
       auto rankVal = rewriter.create<LLVM::ConstantOp>(
           loc, getIndexType(), rewriter.getIndexAttr(rank));
-      // undef = UndefOp
+      // poison = PoisonOp
       UnrankedMemRefDescriptor memRefDesc =
-          UnrankedMemRefDescriptor::undef(rewriter, loc, targetStructType);
-      // d1 = InsertValueOp undef, rank, 0
+          UnrankedMemRefDescriptor::poison(rewriter, loc, targetStructType);
+      // d1 = InsertValueOp poison, rank, 0
       memRefDesc.setRank(rewriter, loc, rankVal);
       // d2 = InsertValueOp d1, ptr, 1
       memRefDesc.setMemRefDescPtr(rewriter, loc, ptr);
@@ -928,7 +928,7 @@ struct MemorySpaceCastOpLowering
       Value sourceUnderlyingDesc = sourceDesc.memRefDescPtr(rewriter, loc);
 
       // Create and allocate storage for new memref descriptor.
-      auto result = UnrankedMemRefDescriptor::undef(
+      auto result = UnrankedMemRefDescriptor::poison(
           rewriter, loc, typeConverter->convertType(resultTypeU));
       result.setRank(rewriter, loc, rank);
       SmallVector<Value, 1> sizes;
@@ -1058,7 +1058,7 @@ struct MemRefReinterpretCastOpLowering
 
     // Create descriptor.
     Location loc = castOp.getLoc();
-    auto desc = MemRefDescriptor::undef(rewriter, loc, llvmTargetDescriptorTy);
+    auto desc = MemRefDescriptor::poison(rewriter, loc, llvmTargetDescriptorTy);
 
     // Set allocated and aligned pointers.
     Value allocatedPtr, alignedPtr;
@@ -1128,7 +1128,7 @@ struct MemRefReshapeOpLowering
       // Create descriptor.
       Location loc = reshapeOp.getLoc();
       auto desc =
-          MemRefDescriptor::undef(rewriter, loc, llvmTargetDescriptorTy);
+          MemRefDescriptor::poison(rewriter, loc, llvmTargetDescriptorTy);
 
       // Set allocated and aligned pointers.
       Value allocatedPtr, alignedPtr;
@@ -1210,7 +1210,7 @@ struct MemRefReshapeOpLowering
 
     // Create the unranked memref descriptor that holds the ranked one. The
     // inner descriptor is allocated on stack.
-    auto targetDesc = UnrankedMemRefDescriptor::undef(
+    auto targetDesc = UnrankedMemRefDescriptor::poison(
         rewriter, loc, typeConverter->convertType(targetType));
     targetDesc.setRank(rewriter, loc, resultRank);
     SmallVector<Value, 4> sizes;
@@ -1366,7 +1366,7 @@ class TransposeOpLowering : public ConvertOpToLLVMPattern<memref::TransposeOp> {
     if (transposeOp.getPermutation().isIdentity())
       return rewriter.replaceOp(transposeOp, {viewMemRef}), success();
 
-    auto targetMemRef = Mem...
[truncated]

Copy link
Member

@kuhar kuhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me. I do not have much stake in the LLVM dialect, so please wait for another approval.

Copy link
Contributor

@dcaballe dcaballe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG, thanks! I'm also working on introducing poison at some MLIR levels so this is very timely... Thanks!

Please, wait for more reviewers to chime in.

@krzysz00
Copy link
Contributor Author

krzysz00 commented Feb 5, 2025

Ok, given that we've got several approvals, including one from someone who knows the NVVM lowerings (and didn't include a "please wait for more people" comment) I plan to land this this US afternoon

@nikic
Copy link
Contributor

nikic commented Feb 5, 2025

Can you please confirm that this does not introduce any poison uses that may end up being stored in memory?

@krzysz00
Copy link
Contributor Author

krzysz00 commented Feb 6, 2025

@nikic I went and looked, and, as best I can tell, everything that's about to become poison is being used as the initial value for a struct or some other object that'll be fully populated immediately afterwards.

(I did find one use that flows into a bitcast that was unclear enough that I swapped it back to undef, though, so good question)

@nikic
Copy link
Contributor

nikic commented Feb 6, 2025

@nikic I went and looked, and, as best I can tell, everything that's about to become poison is being used as the initial value for a struct or some other object that'll be fully populated immediately afterwards.

Thanks for confirming!

(I did find one use that flows into a bitcast that was unclear enough that I swapped it back to undef, though, so good question)

I think that one's fine: The bitcast is on the inserted value, not the vector.

@krzysz00 krzysz00 merged commit f4e3b87 into llvm:main Feb 6, 2025
6 of 7 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 6, 2025

LLVM Buildbot has detected a new failure on builder ppc64le-flang-rhel-clang running on ppc64le-flang-rhel-test while building mlir at step 6 "test-build-unified-tree-check-flang".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/19372

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-flang) failure: test (failure)
******************** TEST 'Flang :: Lower/PowerPC/ppc-vec-splat-elem-order.f90' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/bin/flang -fc1 -flang-experimental-hlfir -emit-llvm /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-splat-elem-order.f90 -fno-ppc-native-vector-element-order -triple ppc64le-unknown-linux -o - | /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/bin/FileCheck --check-prefixes="LLVMIR" /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-splat-elem-order.f90
+ /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/bin/flang -fc1 -flang-experimental-hlfir -emit-llvm /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-splat-elem-order.f90 -fno-ppc-native-vector-element-order -triple ppc64le-unknown-linux -o -
+ /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/bin/FileCheck --check-prefixes=LLVMIR /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-splat-elem-order.f90
/home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-splat-elem-order.f90:11:11: error: LLVMIR: expected string not found in input
! LLVMIR: %[[ins:.*]] = insertelement <4 x float> undef, float %[[ele]], i32 0
          ^
<stdin>:9:43: note: scanning from here
 %4 = extractelement <4 x float> %3, i64 3
                                          ^
<stdin>:9:43: note: with "ele" equal to "4"
 %4 = extractelement <4 x float> %3, i64 3
                                          ^
<stdin>:10:2: note: possible intended match here
 %5 = insertelement <4 x float> poison, float %4, i32 0
 ^

Input file: <stdin>
Check file: /home/buildbots/llvm-external-buildbots/workers/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Lower/PowerPC/ppc-vec-splat-elem-order.f90

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            1: ; ModuleID = 'FIRModule' 
            2: source_filename = "FIRModule" 
            3: target datalayout = "e-m:e-Fn32-i64:64-i128:128-n32:64-S128-v256:256:256-v512:512:512" 
            4: target triple = "ppc64le-unknown-linux" 
            5:  
            6: define void @vec_splat_testf32i64_(ptr captures(none) %0) #0 { 
            7:  %2 = alloca <4 x float>, i64 1, align 16 
            8:  %3 = load <4 x float>, ptr %0, align 16 
            9:  %4 = extractelement <4 x float> %3, i64 3 
check:11'0                                               X error: no match found
check:11'1                                                 with "ele" equal to "4"
           10:  %5 = insertelement <4 x float> poison, float %4, i32 0 
check:11'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:11'2      ?                                                       possible intended match
           11:  %6 = shufflevector <4 x float> %5, <4 x float> poison, <4 x i32> zeroinitializer 
check:11'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           12:  store <4 x float> %6, ptr %2, align 16 
check:11'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           13:  ret void 
check:11'0     ~~~~~~~~~~
           14: } 
check:11'0     ~~
...

@kuhar
Copy link
Member

kuhar commented Feb 6, 2025

@krzysz00 seems like some flang tests need to be updated

Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…vm#125629)

LLVM itself is generally moving away from using `undef` and towards
using `poison`, to the point of having a lint that caches new uses of
`undef` in tests.

In order to not trip the lint on new patterns and to conform to the
evolution of LLVM
- Rename valious ::undef() methods on StructBuilder subclasses to
::poison()
- Audit the uses of UndefOp in the MLIR libraries and replace almost all
of them with PoisonOp

The remaining uses of `undef` are initializing `uninitialized` memrefs,
explicit conversions to undef from SPIR-V, and a few cases in
AMDGPUToROCDL where usage like

    %v = insertelement <M x iN> undef, iN %v, i32 0
    %arg = bitcast <M x iN> %v to i(M * N)

is used to handle "i32" arguments that are are really packed vectors of
smaller types that won't always be fully initialized.
paul0403 added a commit to PennyLaneAI/catalyst that referenced this pull request May 28, 2025
1. Moving ops around due to random ordering updates
2. Revert the inserted unrealized_conversion_casts in #1751
3. llvm.mlir.undef is deprecated, use llvm.mlir.poison instead
llvm/llvm-project#125629
paul0403 added a commit to PennyLaneAI/catalyst that referenced this pull request Jun 3, 2025
**Context:**
We update the llvm version tagged by jax 0.6.0:
```
mhlo=617a9361d186199480c080c9e8c474a5e30c22d1
llvm=179d30f8c3fddd3c85056fd2b8e877a4a8513158
```

We also update Enzyme to the latest version, which is 0.0.180, at commit
`db0181320d6e425ee963bd496ed0d8dbb615be18`


**Description of the Change:**

Firstly, jax recently moved from the Google github organization to its
own jax-ml organization.
This means the urls, and the retrieval method for the underlying llvm
and mhlo git commit tags, needs to be updated. (Thanks @mehrdad2m !)

Now on to the actual changes. I will list the changes in increasing
complexity.
1. The new enzyme cmake target is `EnzymeStatic-21` (from 20)

2. Enzyme works with a later llvm then our target, so it has some llvm
intrinsics unknown to the one we are targeting. We patch them away. They
do not concern us since they are all intrinsics for nvidia backends.

3. `applyPatternsAndFoldGreedily` is removed. Drop-in replacement is
`applyPatternsGreedily`.
llvm/llvm-project#104649,
llvm/llvm-project#126701

4. ops with `CallOpInterface` must have two new optional attributes
`arg_attrs` and `res_attrs`
llvm/llvm-project#123176

5. `CallInterfaceCallable` objects now must be directly casted to the
callee `SymbolRefAttr`, i.e. `callee.get<SymbolRefAttr>()` ->
`cast<SymbolRefAttr>(callee)`
llvm/llvm-project@35e8989

6. The `lookupOrCreateFn` family of functions now return
`FailureOr<funcop>` instead of just `funcop`, so a `.value()` needs to
be used to retrieve the underlying `funcop`.
llvm/llvm-project@e84f6b6

7. The cpp api for `OneShotBufferizePassOptions` no longer needs
complicated lambdas for the type converter options. They can be set with
the `mlir::bufferization::LayoutMapOption::IdentityLayoutMap` options
directly.

8. The individual `match` and `rewrite` methods in pattern rewrites are
removed. Use the two-in-one `matchAndRewrite` instead.
llvm/llvm-project#129861

9. For rewrite patterns with 1-to-N convertions, a new `macthAndRewrite`
overload with `OneToNOpAdaptor` must be used. For us, this is only the
`catalyst.list*` ops. llvm/llvm-project#116470

10. The lowering of `cf::AssertOp` to llvm was split from the
overall`--covert-cf-to-llvm` pass. We need to manually call this
separate pattern for cf.assert duriing quantum to llvm dialect lowering,
where we also convert cf to llvm.
https://github.com/llvm/llvm-project/pull/120431/files 

11. The new mhlo depends on a
[shardy](https://github.com/openxla/shardy) dialect. Shardy is built
with bazel, not cmake. Building shardy ourselves would be very difficult
(not having bazel in our build ecosystem is a hard constraint, cc @mlxd
), and also not necessary (we just use mhlo for their "standard"
passes). We thus patch out all shardy components.

12. Three necessary passes were removed in mhlo:
`mhlo-legalize-control-flow`, `mhlo-legalize-to-std`,
`hlo-legalize-sort`
tensorflow/mlir-hlo@4a640be#diff-ef0d7e30da19a396ba036405a9ef636f8b1be194618b0a90f4602671fc2ef34d

tensorflow/mlir-hlo@2a5e267#diff-f8c7cb07b43593403e00e0dbf9983f0186b4eb70368cc99af3b924061f1ea46f
- Alongside the removal of `mhlo-legalize-to-std`, the cmake target
`MhloToStandard` was removed too.
  We simply patch them back for now. 
  
**For the above two points, note that there will be an overall migration
to the stablehlo repo, as mhlo is sunseting.
Therefore, spending too much time on this isn't necessary, so we just
patch.**

13. The new pattern applicator (`applyPatternsGreedily`) is more
aggressive in dead code elimination,
and is eliminating dead `Value`s in the adjoint gradient method.
The `nodealloc` function we generate for adjoint gradient lowering used
to only return the qreg, not the expval result. This causes the expval
op to be eliminated since it has no users.
This further causes wrong gradient results, since the entire program,
all ops included (regardless
of dead or not), impacts the gradient through chain rule.
To avoid this, we return the expval result as well.
In doing this, we implicitly assume that differentiated qnodes can only
return expval.
Although this assumption is true and also restricted by frontend,
ideally we should not have it hard coded.
We leave this as a TODO for a future feature.

14. The old `--buffer-deallocation` pass is removed. Intended
replacement is `--buffer-deallocation-pipeline`.
This migration is very complicated. We simply add back the old buffer
deallocation pass in the catalyst dialect as a util for now.
We will revisit this in #1778 . 


mlir lit test updates:
1. `bufferization.to_tensor/memref` updated assembly format
2. gradient adjoint lowering test returns both qreg and expval
3. Some inverse unrealized conversion cast pairs are canceled by the new
pattern rewriter.
4. `llvm.mlir.undef` is deprecated, use `llvm.mlir.poison` instead.
llvm/llvm-project#125629


**Benefits:**
Up to date with upstream versions.


[sc-92017]

---------

Co-authored-by: Tzung-Han Juang <tzunghan.juang@gmail.com>
Co-authored-by: Ritu Thombre <42207923+ritu-thombre99@users.noreply.github.com>
Co-authored-by: Mehrdad Malekmohammadi <mehrdad.malek@xanadu.ai>
Co-authored-by: Mehrdad Malek <39844030+mehrdad2m@users.noreply.github.com>
Co-authored-by: David Ittah <dime10@users.noreply.github.com>
Co-authored-by: Joey Carter <joseph.carter@xanadu.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants