[NFCI][IR] Add optional DataLayout argument to zero and null value related APIs

[shiltian]

When the semantics of ConstantPointerNull change to represent a semantic null pointer in the future, a null value won't necessarily be a zero value anymore. Because of that, the entire LLVM constant infrastructure will need to change. As a first step, this PR adds an optional data layout pointer to isNullValue, isZeroValue, getNullValue, and getZeroValue.

---

This PR is with help with AI but I reviewed all the code changes.

[shiltian]

• [WIP][IR][Constants] Change the semantic of ConstantPointerNull to represent an actual nullptr instead of a zero-value pointer #183215
• [NFC] Pass DataLayout to Constant::getNullValue call sites #183214
• [NFC] Thread DataLayout through helper function signatures for aggregate construction #183213
• [NFC] Pass DataLayout to aggregate constant factory call sites #183212
• [NFCI][IR] Thread DataLayout through aggregate construction for collapse checks #183211
• [NFCI] Use DataLayout::getNullPtrValue in codegen and analysis paths #183210
• [NFCI][IR] Thread DataLayout through ConstantFold; fix CAZ extraction and aggregate collapse #183209
• [NFCI][IR] Add optional DataLayout argument to zero and null value related APIs #183208 👈 (View in Graphite)
• [RFC][DataLayout] Add null pointer value infrastructure #183207
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Shilei Tian (shiltian)

Changes

Modify Constant::isZeroValue() and Constant::getNullValue() to accept an
optional const DataLayout *DL = nullptr parameter, and add a new
Constant::getZeroValue() factory method. This establishes the API
distinction between "null value" (semantic null pointer, which may be
non-zero on some targets) and "zero value" (all-zero bits).

When DataLayout is provided:

• isZeroValue() checks ConstantPointerNull against the target's null
  pointer bit pattern via DL->isNullPointerAllZeroes(AS), returning
  false for address spaces where null is not zero.
• getNullValue() constructs aggregates element-by-element when they
  contain pointer elements in non-zero-null address spaces, preserving
  ConstantPointerNull instead of collapsing to ConstantAggregateZero.

When DataLayout is not provided, both functions behave identically to
their previous implementations, ensuring full backward compatibility.

getZeroValue() currently delegates to getNullValue() and will return a
distinct zero-valued pointer constant (via inttoptr) once constant
folding is made DataLayout-aware in a follow-up patch.

---

Full diff: https://github.com/llvm/llvm-project/pull/183208.diff

3 Files Affected:

• (modified) llvm/include/llvm/IR/Constant.h (+14-1)
• (modified) llvm/lib/IR/Constants.cpp (+27-2)
• (modified) llvm/unittests/IR/ConstantsTest.cpp (+124)

diff --git a/llvm/include/llvm/IR/Constant.h b/llvm/include/llvm/IR/Constant.h
index 82a570e8a1446..be0cc6b5665d4 100644
--- a/llvm/include/llvm/IR/Constant.h
+++ b/llvm/include/llvm/IR/Constant.h
@@ -22,6 +22,7 @@ namespace llvm {
 
 class ConstantRange;
 class APInt;
+class DataLayout;
 
 /// This is an important base class in LLVM. It provides the common facilities
 /// of all constant values in an LLVM program. A constant is a value that is
@@ -69,6 +70,9 @@ class Constant : public User {
   /// getZeroValueForNegation.
   LLVM_ABI bool isNegativeZeroValue() const;
 
+  /// Return true iff this constant has an all-zero bit pattern.
+  LLVM_ABI bool isZeroValue(const DataLayout *DL = nullptr) const;
+
   /// Return true if the value is not the smallest signed value, or,
   /// for vectors, does not contain smallest signed value elements.
   LLVM_ABI bool isNotMinSignedValue() const;
@@ -187,7 +191,16 @@ class Constant : public User {
   ///
   LLVM_ABI void handleOperandChange(Value *, Value *);
 
-  LLVM_ABI static Constant *getNullValue(Type *Ty);
+  /// Constructor to create a null constant of arbitrary type.
+  /// Currently equivalent to getZeroValue(). Will diverge once pointer null
+  /// semantics change: for pointer types in address spaces with non-zero null,
+  /// getNullValue() will return the semantic null pointer (ConstantPointerNull)
+  /// while getZeroValue() will continue to return the all-zero-bits value.
+  LLVM_ABI static Constant *getNullValue(Type *Ty,
+                                         const DataLayout *DL = nullptr);
+
+  /// Return the all-zero-bits constant for the given type.
+  LLVM_ABI static Constant *getZeroValue(Type *Ty);
 
   /// @returns the value for an integer or vector of integer constant of the
   /// given type that has all its bits set to true.
diff --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp
index 78ac276f4f3da..5a011f50ef94b 100644
--- a/llvm/lib/IR/Constants.cpp
+++ b/llvm/lib/IR/Constants.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/ConstantFold.h"
+#include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GetElementPtrTypeIterator.h"
@@ -71,6 +72,27 @@ bool Constant::isNegativeZeroValue() const {
   return isNullValue();
 }
 
+// Return true iff this constant has an all-zero bit pattern.
+bool Constant::isZeroValue(const DataLayout *DL) const {
+  // When DataLayout is available, check if ConstantPointerNull actually has
+  // a zero bit pattern (it might not for non-zero-null address spaces).
+  if (DL) {
+    if (const auto *CPN = dyn_cast<ConstantPointerNull>(this))
+      return DL->isNullPointerAllZeroes(CPN->getType()->getAddressSpace());
+
+    // Check for vector splats of ConstantPointerNull.
+    if (getType()->isVectorTy()) {
+      if (const auto *SplatCPN =
+              dyn_cast_or_null<ConstantPointerNull>(getSplatValue())) {
+        return DL->isNullPointerAllZeroes(
+            SplatCPN->getType()->getAddressSpace());
+      }
+    }
+  }
+
+  return this == getZeroValue(getType());
+}
+
 bool Constant::isNullValue() const {
   // 0 is null.
   if (const ConstantInt *CI = dyn_cast<ConstantInt>(this))
@@ -370,8 +392,11 @@ bool Constant::containsConstantExpression() const {
   return false;
 }
 
-/// Constructor to create a '0' constant of arbitrary type.
-Constant *Constant::getNullValue(Type *Ty) {
+Constant *Constant::getNullValue(Type *Ty, const DataLayout *DL) {
+  return getZeroValue(Ty);
+}
+
+Constant *Constant::getZeroValue(Type *Ty) {
   switch (Ty->getTypeID()) {
   case Type::IntegerTyID:
     return ConstantInt::get(Ty, 0);
diff --git a/llvm/unittests/IR/ConstantsTest.cpp b/llvm/unittests/IR/ConstantsTest.cpp
index 34898aa467788..96730dbf16758 100644
--- a/llvm/unittests/IR/ConstantsTest.cpp
+++ b/llvm/unittests/IR/ConstantsTest.cpp
@@ -10,6 +10,7 @@
 #include "llvm-c/Core.h"
 #include "llvm/AsmParser/Parser.h"
 #include "llvm/IR/ConstantFold.h"
+#include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
@@ -868,5 +869,128 @@ TEST(ConstantsTest, Float128Test) {
   LLVMContextDispose(C);
 }
 
+TEST(ConstantsTest, ZeroValueAPIs) {
+  LLVMContext Context;
+
+  // Basic types.
+  Type *Int32Ty = Type::getInt32Ty(Context);
+  Type *FloatTy = Type::getFloatTy(Context);
+  Type *PtrTy = PointerType::get(Context, 0);
+  Type *Ptr1Ty = PointerType::get(Context, 1);
+
+  // --- getZeroValue: currently returns same as getNullValue ---
+  EXPECT_EQ(Constant::getZeroValue(Int32Ty), Constant::getNullValue(Int32Ty));
+  EXPECT_EQ(Constant::getZeroValue(FloatTy), Constant::getNullValue(FloatTy));
+  EXPECT_EQ(Constant::getZeroValue(PtrTy), Constant::getNullValue(PtrTy));
+  EXPECT_EQ(Constant::getZeroValue(Ptr1Ty), Constant::getNullValue(Ptr1Ty));
+
+  // Aggregate types.
+  StructType *StructTy = StructType::get(Int32Ty, PtrTy);
+  ArrayType *ArrayTy = ArrayType::get(Int32Ty, 4);
+  EXPECT_EQ(Constant::getZeroValue(StructTy), Constant::getNullValue(StructTy));
+  EXPECT_EQ(Constant::getZeroValue(ArrayTy), Constant::getNullValue(ArrayTy));
+
+  // --- isZeroValue(nullptr): identity check against getZeroValue ---
+  Constant *IntZero = ConstantInt::get(Int32Ty, 0);
+  Constant *IntOne = ConstantInt::get(Int32Ty, 1);
+  Constant *FPZero = ConstantFP::get(FloatTy, 0.0);
+  Constant *FPNegZero = ConstantFP::get(FloatTy, -0.0);
+  Constant *FPOne = ConstantFP::get(FloatTy, 1.0);
+  Constant *PtrNull0 = ConstantPointerNull::get(cast<PointerType>(PtrTy));
+  Constant *PtrNull1 = ConstantPointerNull::get(cast<PointerType>(Ptr1Ty));
+  Constant *CAZ = ConstantAggregateZero::get(StructTy);
+
+  EXPECT_TRUE(IntZero->isZeroValue());
+  EXPECT_FALSE(IntOne->isZeroValue());
+  EXPECT_TRUE(FPZero->isZeroValue());
+  // -0.0 has a non-zero bit pattern (sign bit set), so it is NOT a zero value.
+  EXPECT_FALSE(FPNegZero->isZeroValue());
+  EXPECT_FALSE(FPOne->isZeroValue());
+  EXPECT_TRUE(PtrNull0->isZeroValue());
+  EXPECT_TRUE(PtrNull1->isZeroValue());
+  EXPECT_TRUE(CAZ->isZeroValue());
+
+  // --- isZeroValue: FP corner cases ---
+  // -0.0 is NOT zero (sign bit set = non-zero bit pattern).
+  // Verify consistency with isNullValue: both agree +0.0 is zero, -0.0 is not.
+  EXPECT_TRUE(FPZero->isNullValue());
+  EXPECT_FALSE(FPNegZero->isNullValue());
+  EXPECT_TRUE(FPZero->isZeroValue());
+  EXPECT_FALSE(FPNegZero->isZeroValue());
+
+  // Double precision: same behavior.
+  Type *DoubleTy = Type::getDoubleTy(Context);
+  Constant *DblZero = ConstantFP::get(DoubleTy, 0.0);
+  Constant *DblNegZero = ConstantFP::get(DoubleTy, -0.0);
+  EXPECT_TRUE(DblZero->isZeroValue());
+  EXPECT_FALSE(DblNegZero->isZeroValue());
+
+  // Vector splats of FP zeros.
+  Constant *VecPosZero =
+      ConstantVector::getSplat(ElementCount::getFixed(2), FPZero);
+  Constant *VecNegZero =
+      ConstantVector::getSplat(ElementCount::getFixed(2), FPNegZero);
+  // Splat of +0.0 collapses to CAZ, which is zero.
+  EXPECT_TRUE(isa<ConstantAggregateZero>(VecPosZero));
+  EXPECT_TRUE(VecPosZero->isZeroValue());
+  // Splat of -0.0 does NOT collapse to CAZ and is NOT zero.
+  EXPECT_FALSE(isa<ConstantAggregateZero>(VecNegZero));
+  EXPECT_FALSE(VecNegZero->isZeroValue());
+
+  // --- isZeroValue(&DL) with default DataLayout (all AS have zero null) ---
+  DataLayout DefaultDL("");
+  EXPECT_TRUE(IntZero->isZeroValue(&DefaultDL));
+  EXPECT_FALSE(IntOne->isZeroValue(&DefaultDL));
+  EXPECT_TRUE(FPZero->isZeroValue(&DefaultDL));
+  EXPECT_FALSE(FPNegZero->isZeroValue(&DefaultDL));
+  EXPECT_FALSE(FPOne->isZeroValue(&DefaultDL));
+  EXPECT_TRUE(PtrNull0->isZeroValue(&DefaultDL));
+  EXPECT_TRUE(PtrNull1->isZeroValue(&DefaultDL));
+  EXPECT_TRUE(CAZ->isZeroValue(&DefaultDL));
+
+  // --- isZeroValue(&DL) with all-ones-null AS 1 ---
+  // Format: p<flags><as>:<size>:<abi> -- flags before AS number.
+  DataLayout AllOnesDL("po1:64:64");
+  // AS 0 still has zero null, so CPN for AS 0 is still a zero value.
+  EXPECT_TRUE(PtrNull0->isZeroValue(&AllOnesDL));
+  // AS 1 has all-ones null, so CPN for AS 1 is NOT a zero value.
+  EXPECT_FALSE(PtrNull1->isZeroValue(&AllOnesDL));
+  // Non-pointer constants are unaffected by DataLayout.
+  EXPECT_TRUE(IntZero->isZeroValue(&AllOnesDL));
+  EXPECT_TRUE(FPZero->isZeroValue(&AllOnesDL));
+  EXPECT_TRUE(CAZ->isZeroValue(&AllOnesDL));
+
+  // --- getNullValue(Ty, nullptr): same as getNullValue(Ty) ---
+  EXPECT_EQ(Constant::getNullValue(Int32Ty, nullptr),
+            Constant::getNullValue(Int32Ty));
+  EXPECT_EQ(Constant::getNullValue(PtrTy, nullptr),
+            Constant::getNullValue(PtrTy));
+  EXPECT_EQ(Constant::getNullValue(StructTy, nullptr),
+            Constant::getNullValue(StructTy));
+
+  // --- getNullValue(Ty, &DL) fast path: no non-zero-null pointers ---
+  EXPECT_EQ(Constant::getNullValue(Int32Ty, &DefaultDL),
+            Constant::getNullValue(Int32Ty));
+  EXPECT_EQ(Constant::getNullValue(PtrTy, &DefaultDL),
+            Constant::getNullValue(PtrTy));
+  EXPECT_EQ(Constant::getNullValue(StructTy, &DefaultDL),
+            Constant::getNullValue(StructTy));
+  EXPECT_EQ(Constant::getNullValue(ArrayTy, &DefaultDL),
+            Constant::getNullValue(ArrayTy));
+
+  // With AllOnesDL, types that don't contain AS 1 pointers still take fast
+  // path.
+  EXPECT_EQ(Constant::getNullValue(Int32Ty, &AllOnesDL),
+            Constant::getNullValue(Int32Ty));
+  EXPECT_EQ(Constant::getNullValue(PtrTy, &AllOnesDL),
+            Constant::getNullValue(PtrTy));
+  // Struct containing AS 0 pointer -- fast path (AS 0 is zero null).
+  EXPECT_EQ(Constant::getNullValue(StructTy, &AllOnesDL),
+            Constant::getNullValue(StructTy));
+
+  // TODO: getNullValue slow path for aggregates with non-zero-null pointers is
+  // deferred to PR 3 testing (requires aggregate collapse fix).
+}
+
 } // end anonymous namespace
 } // end namespace llvm

[github-actions]

🐧 Linux x64 Test Results

• 194652 tests passed
• 5175 tests skipped

✅ The build succeeded and all tests passed.

[arichardson]

Nit: I'd change the commit title to say "optional DataLayout argument" instead of DataLayout pointer.

[github-actions]

⚠️ undef deprecator found issues in your code. ⚠️

You can test this locally with the following command:

git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef([^a-zA-Z0-9_-]|$)|UndefValue::get)' 'HEAD~1' HEAD llvm/include/llvm/IR/Constant.h llvm/lib/IR/Constants.cpp llvm/unittests/IR/ConstantsTest.cpp

The following files introduce new uses of undef:

• llvm/unittests/IR/ConstantsTest.cpp

Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields undef. You should use poison values for placeholders instead.

In tests, avoid using undef and having tests that trigger undefined behavior. If you need an operand with some unimportant value, you can add a new argument to the function and use that instead.

For example, this is considered a bad practice:

define void @fn() {
  ...
  br i1 undef, ...
}

Please use the following instead:

define void @fn(i1 %cond) {
  ...
  br i1 %cond, ...
}

Please refer to the Undefined Behavior Manual for more information.

[shiltian]

@arichardson I updated the PR to explicitly handle isNullValue and isZeroValue. getNullValue and getZeroValue are not changed, because we don't change the semantics yet, so we don't need to diverge the representation at this moment.

[nikic]

Why does isNullValue() need the DataLayout argument as well? Wouldn't that always be represented by ConstantPointerNull?

[shiltian]

There might be other types that contain pointers indirectly as well?

[shiltian]

are you suggesting that we don't actually need to check recursively?

[shiltian]

well, we have to though, because the current getNullValue will simply return ConstantAggregateZero for any aggregate types, but this will no longer be the case after we change the semantics. We will have to recursively construct null/zero value accordingly.

[nikic]

Checking for an all-null-pointer struct/array probably doesn't make a lot of sense (unlike an all-zero struct/array). I think the only use case would be for a vector of null pointers, which does seem potentially meaningful.

Though after null pointers are not longer always all-zero, I do wonder whether we should be using ConstantAggregateZero for a vector of null pointer, rather than using a ConstantPointerNull with vector type (similar to how we're in the process of switching ConstantFP/ConstantInt to natively support vector splats. This has recently been enabled by default for FP). This has the benefit that we can always represent a null pointer splat, regardless of whether the null pointer is all zero or not.

[shiltian]

Checking for an all-null-pointer struct/array probably doesn't make a lot of sense (unlike an all-zero struct/array).

Hmm, I wonder what the expected behavior would be if we check isNullValue and isZeroValue on things like { i32, ptr, i32 }.

Though after null pointers are not longer always all-zero, I do wonder whether we should be using ConstantAggregateZero for a vector of null pointer, rather than using a ConstantPointerNull with vector type (similar to how we're in the process of switching ConstantFP/ConstantInt to natively support vector splats. This has recently been enabled by default for FP). This has the benefit that we can always represent a null pointer splat, regardless of whether the null pointer is all zero or not.

That is a great idea! I went ahead doing it right now. #195486

[shiltian]

After we change the semantics, this should return false. The challenging part in the future would be in those places where DL is not really available, like the constant folding (not the pass).

[arichardson]

I think longer term it would be good if we only allow this function to be used for pointer types (and vectors of pointers).

[nikic]

Another high level question I have is whether we should be making a harder distinction between the cases where we need the DL and where we don't. In particular, a very large number of current isNullValue() checks are only operating on integer or vector of integer types, and in that case DL is never needed.

So I wonder whether having an isZeroInt() API that doesn't need DL makes sense, or whether isZeroValue() with optional DL is good enough. The general direction of the thought it that we could make DL required in the cases where it matters, but making it globally required would be annoying for cases where it's irrelevant.

[arsenm]

Ideally we would end up with no optional DL arguments

[shiltian]

I think the ideas from @nikic and @arichardson can converge on a similar direction, which is to narrow the usage and scope of isNullValue. I took a small step further in #195486, which should make it easier to check for vectors of null pointers in the future.

[shiltian]

Does it make sense that we change all existing uses of isNullValue to isZeroValue first, and then add back isNullValue along with the semantics change of ConstantPointerNull only for pointers (as well as vector of pointers)?

[arsenm]

Does it make sense that we change all existing uses of isNullValue to isZeroValue first, and then add back isNullValue along with the semantics change of ConstantPointerNull only for pointers (as well as vector of pointers)?

I think so (though I would maybe name it isNullPtrValue? Though wouldn't that only be covered by ConstantPointerNull?)

[shiltian]

The change for ConstantPointerNull to represent both scalar and vector was merged. I think we can safely just check against ConstantPointerNull for most of the cases. However, I'm still not sure about the remaining small part, as I have asked in #183208 (comment): what would be the result of isNullValue and isZeroValue on things like { i32, ptr, i64}? I suppose isZeroValue is fairly simple: check against ConstantAggregateZero, but for isNullValue (or isNullPtr or whatever name we will use in the future), I suppose we still need to check recursively? Or we simply just say, isNullX is only valid for pointers?

[nikic]

Depends a bit on what usages you find, but I'd be partial to not having isNullValue() at all and using isa<ConstantPointerNull>() instead...