[WIP][IR][Constants] Change the semantic of `ConstantPointerNull` to represent an actual `nullptr` instead of a zero-value pointer #166667

shiltian · 2025-11-05T23:31:09Z

Motivation

The value of a nullptr is not always 0. For example, on AMDGPU, the nullptr in address spaces 3 and 5 is 0xffffffff. Currently, there is no target-independent way to get this information, making it difficult and error-prone to handle null pointers in target-agnostic code.

We do have ConstantPointerNull, but it might be a little confusing and misleading. It represents a pointer with an all-zero value rather than necessarily a real nullptr. Therefore, to represent a real nullptr in address space N, we need to use addrspacecast ptr null to ptr addrspace(N) and it can't be folded.

Proposal

In this PR, we change the semantic of ConstantPointerNull to represent an actual nullptr instead of a zero-value pointer. Here is the detailed changes.

IR

ptr addrspace(N) null will represent the actual nullptr in address space N.
ptr addrspace(N) zeroinitializer will represent a zero-value pointer in address space N.

API

Constant::getNullValue will return a null value. It is same as the current semantics except for the PointerType, which will return a real nullptr pointer.
Constant::getZeroValue will return a zero value constant. It is completely same as the current semantics. To represent a zero-value pointer, a ConstantExpr will be used (effectively inttoptr i8 0 to ptr addrspace(N)).
Correspondingly, there will be both Constant::isNullValue and Constant::isZeroValue.

The RFC is https://discourse.llvm.org/t/rfc-introduce-sentinel-pointer-value-to-datalayout/85265. It is a little bit old and the title might look different, but everything eventually converges to this change. An early attempt can be found in #131557, which has many valuable discussion as well.

This PR is still WIP but any early feedback is welcome. I'll include as many necessary code changes as possible in this PR, but eventually this needs to be carefully split into multiple PRs, and I'll do it after the changes look good to every one.

shiltian · 2025-11-05T23:31:28Z

[WIP][IR][Constants] Change the semantic of ConstantPointerNull to represent an actual nullptr instead of a zero-value pointer #166667 👈 (View in Graphite)
main

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

llvm/lib/IR/ConstantFold.cpp

llvm/lib/IR/Constants.cpp

llvm/lib/IR/ConstantFold.cpp

llvm/lib/Analysis/ConstantFolding.cpp

llvm/lib/IR/Constants.cpp

arichardson · 2025-11-06T18:33:46Z

llvm/lib/IR/DataLayout.cpp


 // Default pointer type specifications.
-constexpr DataLayout::PointerSpec DefaultPointerSpecs[] = {
+const DataLayout::PointerSpec DefaultPointerSpecs[] = {


We could avoid the need for a global ctor/dtor if we just stored a plain integer value? Since we only support 0/-1 we could just rely on sign-extension? But of course that makes the getter slower so probably best to ignore this suggestion.

I thought about that, but we might want to allow a target to specify that the nullptr is an arbitrary bit pattern to be more future proof such that LLVM will not try any folding regarding those casts involving nullptr. I added 'c' as the flag to specify that.

Well, after a second thought, maybe we can store a random value (not 0/-1) to indicate it is custom?

llvm/test/CodeGen/AMDGPU/addrspacecast.gfx6.ll

arichardson · 2025-11-06T23:21:50Z

llvm/lib/Analysis/ConstantFolding.cpp

                                              /*IsSigned=*/false, DL);
      } else if (auto *GEP = dyn_cast<GEPOperator>(CE)) {
        // If we have GEP, we can perform the following folds:
        // (ptrtoint/ptrtoaddr (gep null, x)) -> x


Doesn't this fold now need to check for Base->isZero() or use the null pointer APInt value as the base?

arichardson · 2025-11-06T23:23:14Z

llvm/lib/Analysis/ConstantFolding.cpp

        auto *Base = cast<Constant>(GEP->stripAndAccumulateConstantOffsets(
            DL, BaseOffset, /*AllowNonInbounds=*/true));
        if (Base->isNullValue()) {
          FoldedValue = ConstantInt::get(CE->getContext(), BaseOffset);


Suggested change

FoldedValue = ConstantInt::get(CE->getContext(), NullPtrValue + BaseOffset);

plus appropriate handling of zero-extension

shiltian · 2025-11-07T05:01:50Z

llvm/test/CodeGen/AMDGPU/addrspacecast.r600.ll

 ; CHECK-NEXT:    ALU clause starting at 4:
 ; CHECK-NEXT:     MOV * T0.X, literal.x,
-; CHECK-NEXT:    -1(nan), 0(0.000000e+00)
+; CHECK-NEXT:    0(0.000000e+00), 0(0.000000e+00)


Does R600 even have multiple address spaces? It doesn't look like the case from the data layout string.

arichardson · 2025-11-07T16:32:41Z

llvm/test/Instrumentation/AddressSanitizer/asan-scalable-vector.ll

-; CHECK-NEXT:    call void @__asan_report_store8(i64 0) #[[ATTR4:[0-9]+]]
+; CHECK-NEXT:    br i1 [[TMP1]], label %[[BB5:.*]], label %[[BB6:.*]]
+; CHECK:       [[BB5]]:
+; CHECK-NEXT:    call void @__asan_report_store8(i64 ptrtoint (ptr null to i64)) #[[ATTR4:[0-9]+]]


I feel like we should really be passing a pointer to these hooks rather than an integer value, but also am surprised this isn't using the datalayout aware constant folding.

arichardson · 2025-11-07T16:35:55Z

llvm/test/Instrumentation/HWAddressSanitizer/hwasan-pass-second-run.ll

 ; CHECK-SAME: (ptr [[A:%.*]]) #[[ATTR0:[0-9]+]] {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[DOTHWASAN_SHADOW:%.*]] = call ptr asm "", "=r,0"(ptr null)
+; CHECK-NEXT:    [[DOTHWASAN_SHADOW:%.*]] = call ptr asm "", "=r,0"(ptr zeroinitializer)


Where is this ptr inttoptr 0 coming from? Shouldn't it be using ConstantPointerNull? That would reduce the diff quite a lot.

Hmm looking at HWAddressSanitizer::getShadowNonTls this does actually need to be an integer converted to a pointer, so it needs to be zero even if null pointers are all ones. The commit message says it's there to avoid rematerializing constants, so maybe it could be omitted for the zero case but I am not too familiar with this code (@pcc).

arichardson · 2025-11-07T16:37:52Z

llvm/test/Transforms/InstCombine/ptrtoint-nullgep.ll

+; LLPARSER-NEXT:    ret i64 ptrtoint (ptr addrspace(1) null to i64)
+;
+; INSTSIMPLIFY-LABEL: define {{[^@]+}}@constant_fold_ptrtoint_gep_zero() {
+; INSTSIMPLIFY-NEXT:    ret i64 ptrtoint (ptr addrspace(1) null to i64)


Hmm, shouldn't we have the datalayout in instsimplify, does this mean we don't use the datalayout aware constant folding logic there?

Yes, there are several places where the handling needs to be updated.

arichardson

This generally looks good to me, just a few comments about the test diffs.

…represent an actual `nullptr` instead of a zero-value pointer The value of a `nullptr` is not always `0`. For example, on AMDGPU, the `nullptr` in address spaces 3 and 5 is `0xffffffff`. Currently, there is no target-independent way to get this information, making it difficult and error-prone to handle null pointers in target-agnostic code. We do have `ConstantPointerNull`, but it might be a little confusing and misleading. It represents a pointer with an all-zero value rather than necessarily a real `nullptr`. Therefore, to represent a real `nullptr` in address space `N`, we need to use `addrspacecast ptr null to ptr addrspace(N)` and it can't be folded. In this PR, we change the semantic of `ConstantPointerNull` to represent an actual `nullptr` instead of a zero-value pointer. Here is the detailed changes. * `ptr addrspace(N) null` will represent the actual `nullptr` in address space `N`. * `ptr addrspace(N) zeroinitializer` will represent a zero-value pointer in address space `N`. * `Constant::getNullValue` will return a _null_ value. It is same as the current semantics except for the `PointerType`, which will return a real `nullptr` pointer. * `Constant::getZeroValue` will return a zero value constant. It is completely same as the current semantics. To represent a zero-value pointer, a `ConstantExpr` will be used (effectively `inttoptr i8 0 to ptr addrspace(N)`). * Correspondingly, there will be both `Constant::isNullValue` and `Constant::isZeroValue`. The RFC is https://discourse.llvm.org/t/rfc-introduce-sentinel-pointer-value-to-datalayout/85265. It is a little bit old and the title might look different, but everything eventually converges to this change. An early attempt can be found in #131557, which has many valuable discussion as well. This PR is still WIP but any early feedback is welcome. I'll include as many necessary code changes as possible in this PR, but eventually this needs to be carefully split into multiple PRs, and I'll do it after the changes look good to every one.

shiltian · 2025-11-17T05:17:25Z

Still WIP. With more code changes, there are more test failures. Fixing them one by one. :-)

arichardson

A few more small comments

arichardson · 2025-11-17T06:49:12Z

llvm/lib/Analysis/ConstantFolding.cpp

+    if (std::optional<APInt> NullPtrValue = DL.getNullPtrValue(
+            DestTy->getScalarType()->getPointerAddressSpace())) {
+      if ((NullPtrValue->isZero() && C->isZeroValue()) ||
+          (NullPtrValue->isAllOnes() && C->isAllOnesValue()))


I think we have to check the width of the input value for all ones since inttoptr zero-extends, so a short value would not end up as canonical nullptr.

arichardson · 2025-11-17T06:53:54Z

llvm/lib/Analysis/ConstantFolding.cpp

+
+    // If the input is a nullptr, we can fold it to the corresponding
+    // nullptr in the destination address space.
+    if (C->isNullValue())


Is this actually true? Are null values in one address space always null in all others? I imagine this is almost always true, but maybe you could have some where this conversion is not valid?

I don't see this property in langref so we should either add it or drop this fold.

Is this actually true? Are null values in one address space always null in all others? I imagine this is almost always true, but maybe you could have some where this conversion is not valid?

This is actually the whole point of this PR. The semantic would be, convert the nullptr in AS X to AS Y, so it would still be a nullptr in AS Y. I don't think the semantic should be convert the value of nullptr in AS X to the corresponding pointer value in AS Y.

I'll clarify this in the LangRef.

Sounds good

arichardson · 2025-11-17T07:04:08Z

llvm/lib/IR/Verifier.cpp

    // cannot be constant.
    if (GV.hasCommonLinkage()) {
-      Check(GV.getInitializer()->isNullValue(),
+      Check(GV.getInitializer()->isZeroValue(),


I wonder if we should add an optional data layout parameter to this function so it can return true for null pointers in the common case?

arichardson · 2025-11-17T07:06:00Z

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

        VectorizedValue = Builder.CreateShuffleVector(
-            VectorizedValue,
-            ConstantVector::getNullValue(VectorizedValue->getType()), Mask);
+            VectorizedValue, Constant::getNullValue(VectorizedValue->getType()),


Should this be getZeroValue?

Suggested change

VectorizedValue, Constant::getNullValue(VectorizedValue->getType()),

VectorizedValue, Constant::getZeroValue(VectorizedValue->getType()),

arichardson · 2025-11-17T18:25:42Z

llvm/test/Assembler/ConstantExprFoldCast.ll

 ; CHECK-NOT: bitcast
 ; CHECK-NOT: trunc
-; CHECK: addrspacecast
-; CHECK: addrspacecast
+; CHECK: ptr addrspace(1) null
+; CHECK: ptr null


Unrelated to this PR but these checks are not great, can we regen this test with UTC and --check-globals and pre-commit that?

arichardson · 2025-11-17T18:33:28Z

llvm/test/Instrumentation/HWAddressSanitizer/hwasan-pass-second-run.ll

 ; CHECK-SAME: (ptr [[A:%.*]]) #[[ATTR0:[0-9]+]] {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[DOTHWASAN_SHADOW:%.*]] = call ptr asm "", "=r,0"(ptr null)
+; CHECK-NEXT:    [[DOTHWASAN_SHADOW:%.*]] = call ptr asm "", "=r,0"(ptr zeroinitializer)


Hmm looking at HWAddressSanitizer::getShadowNonTls this does actually need to be an integer converted to a pointer, so it needs to be zero even if null pointers are all ones. The commit message says it's there to avoid rematerializing constants, so maybe it could be omitted for the zero case but I am not too familiar with this code (@pcc).

arichardson · 2025-11-17T18:34:06Z

llvm/unittests/IR/InstructionsTest.cpp

        LLVMContext Ctx;
        Type *OpVFTy = FixedVectorType::get(IntegerType::getInt1Ty(Ctx), OpVF);
-        Value *Op = ConstantVector::getNullValue(OpVFTy);
+        Value *Op = Constant::getNullValue(OpVFTy);


Suggested change

Value *Op = Constant::getNullValue(OpVFTy);

Value *Op = Constant::getZeroValue(OpVFTy);

arichardson · 2025-11-17T18:35:21Z

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

        VectorizedValue = Builder.CreateShuffleVector(
-            VectorizedValue,
-            ConstantVector::getNullValue(VectorizedValue->getType()), Mask);
+            VectorizedValue, Constant::getNullValue(VectorizedValue->getType()),


Suggested change

VectorizedValue, Constant::getNullValue(VectorizedValue->getType()),

VectorizedValue, Constant::getZeroValue(VectorizedValue->getType()),

arichardson · 2025-11-17T18:37:48Z

llvm/lib/IR/ConstantFold.cpp

+    // We can't fold inttoptr(0) to ConstantPointerNull without checking the
+    // target data layout. However, since data layout is not available here, we
+    // can't do this.
+    if (V->isZeroValue())
+      return nullptr;


Suggested change

// We can't fold inttoptr(0) to ConstantPointerNull without checking the

// target data layout. However, since data layout is not available here, we

// can't do this.

if (V->isZeroValue())

return nullptr;

// Note: We can't fold inttoptr(0) to ConstantPointerNull without checking

// the target data layout since null pointers could also be all-ones.

I don't think we need the early return here? Although I guess it's a cheap check and so might actually be better to have it.

Ah is this because of the V->isNullValue() below? In that case let's keep it.

arichardson · 2025-11-17T18:39:20Z

llvm/lib/IR/ConstantFold.cpp

  if (V->isNullValue() && !DestTy->isX86_AMXTy() &&
      opc != Instruction::AddrSpaceCast)
    return Constant::getNullValue(DestTy);


Suggested change

if (V->isNullValue() && !DestTy->isX86_AMXTy())

return Constant::getNullValue(DestTy);

Addrspacecast was already handled above. Not sure what is special about isX86_AMXTy, but I guess we need it.

arichardson · 2025-11-17T18:41:51Z

llvm/lib/Analysis/ConstantFolding.cpp

+        if (NullPtrValue->isZero()) {
+          return Constant::getZeroValue(DestTy);
+        } else if (NullPtrValue->isAllOnes()) {
+          return ConstantInt::get(
+              DestTy, NullPtrValue->zextOrTrunc(DestTy->getScalarSizeInBits()));
+        } else {
+          llvm_unreachable("invalid nullptr value");
+        }


Suggested change

if (NullPtrValue->isZero()) {

return Constant::getZeroValue(DestTy);

} else if (NullPtrValue->isAllOnes()) {

return ConstantInt::get(

DestTy, NullPtrValue->zextOrTrunc(DestTy->getScalarSizeInBits()));

} else {

llvm_unreachable("invalid nullptr value");

}

assert(NullPtrValue->isZero() || NullPtrValue->isAllOnes());

return ConstantInt::get(

DestTy, NullPtrValue->zextOrTrunc(DestTy->getScalarSizeInBits()));

I believe this is equivalent and a bit simpler.

arichardson · 2025-11-17T18:47:11Z

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

+    if (std::optional<APInt> NullPtrVal =
+            DL.getNullPtrValue(NullPtr->getType()->getPointerAddressSpace())) {
+      AP.OutStreamer->emitIntValue(NullPtrVal->getSExtValue(), Size);
+    } else {
+      // We fall back to the default behavior of emitting a zero value if we
+      // can't get the null pointer value from the data layout.
+      AP.OutStreamer->emitIntValue(0, Size);
+    }


Suggested change

if (std::optional<APInt> NullPtrVal =

DL.getNullPtrValue(NullPtr->getType()->getPointerAddressSpace())) {

AP.OutStreamer->emitIntValue(NullPtrVal->getSExtValue(), Size);

} else {

// We fall back to the default behavior of emitting a zero value if we

// can't get the null pointer value from the data layout.

AP.OutStreamer->emitIntValue(0, Size);

}

APInt NullPtrVal = DL.getNullPtrValue(NullPtr->getType()->getPointerAddressSpace());

AP.OutStreamer->emitIntValue(NullPtrVal.getSExtValue(), Size);

I wonder if we should drop the std::optional from the DataLayout and just always have a defined value? We could then also either drop the 'c' flag or do something like pc{0xaaaaa}3:64:64 to define a AS3 pointer with nullptr value of 0xaaaa?

Silently falling back to zero here seems like it would cause more pain for hypothetical targets with custom null pointers than forcing them to set the datalayout?

shiltian commented Nov 5, 2025

View reviewed changes

llvm/lib/IR/ConstantFold.cpp Outdated Show resolved Hide resolved

shiltian requested review from arichardson, arsenm and nikic November 5, 2025 23:37

shiltian force-pushed the users/shiltian/ptr-zeroinitializer-to-inttoptr branch 3 times, most recently from 27a981c to a3f0889 Compare November 6, 2025 03:46

shiltian changed the title ~~[WIP][IR][Constants] Use ptr addrspace(N) zeroinitializer to represent a zero-value pointer~~ [WIP][IR][Constants] Change the semantic of ConstantPointerNull to represent an actual nullptr instead of a zero-value pointer Nov 6, 2025

shiltian force-pushed the users/shiltian/ptr-zeroinitializer-to-inttoptr branch from a3f0889 to fab5565 Compare November 6, 2025 18:49

arichardson reviewed Nov 6, 2025

View reviewed changes

shiltian force-pushed the users/shiltian/ptr-zeroinitializer-to-inttoptr branch 3 times, most recently from a08a000 to 9478a0d Compare November 6, 2025 21:34

shiltian commented Nov 6, 2025

View reviewed changes

llvm/test/CodeGen/AMDGPU/addrspacecast.gfx6.ll Outdated Show resolved Hide resolved

shiltian force-pushed the users/shiltian/ptr-zeroinitializer-to-inttoptr branch 2 times, most recently from a886714 to c47d8e5 Compare November 6, 2025 22:05

arichardson reviewed Nov 6, 2025

View reviewed changes

shiltian force-pushed the users/shiltian/ptr-zeroinitializer-to-inttoptr branch from c47d8e5 to b9669ba Compare November 7, 2025 04:56

shiltian commented Nov 7, 2025

View reviewed changes

arichardson reviewed Nov 7, 2025

View reviewed changes

shiltian force-pushed the users/shiltian/ptr-zeroinitializer-to-inttoptr branch from b9669ba to c52a3b6 Compare November 17, 2025 05:16

arichardson reviewed Nov 17, 2025

View reviewed changes

shiltian mentioned this pull request Nov 17, 2025

[DataLayout] Add isNullPointerAllZeroes #165314

Closed

arichardson reviewed Nov 17, 2025

View reviewed changes

ro-i mentioned this pull request Nov 17, 2025

[InferAddressSpaces] Handle unconverted ptrmask #140802

Open


	FoldedValue = ConstantInt::get(CE->getContext(), NullPtrValue + BaseOffset);

	VectorizedValue, Constant::getNullValue(VectorizedValue->getType()),
	VectorizedValue, Constant::getZeroValue(VectorizedValue->getType()),

	Value *Op = Constant::getNullValue(OpVFTy);
	Value *Op = Constant::getZeroValue(OpVFTy);


	if (V->isNullValue() && !DestTy->isX86_AMXTy())
	return Constant::getNullValue(DestTy);

[WIP][IR][Constants] Change the semantic of ConstantPointerNull to represent an actual nullptr instead of a zero-value pointer #166667

Are you sure you want to change the base?

[WIP][IR][Constants] Change the semantic of ConstantPointerNull to represent an actual nullptr instead of a zero-value pointer #166667

Conversation

shiltian commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Proposal

IR

API

Uh oh!

shiltian commented Nov 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shiltian Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arichardson Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arichardson left a comment

Choose a reason for hiding this comment

Uh oh!

shiltian commented Nov 17, 2025

Uh oh!

arichardson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

[WIP][IR][Constants] Change the semantic of `ConstantPointerNull` to represent an actual `nullptr` instead of a zero-value pointer #166667

[WIP][IR][Constants] Change the semantic of `ConstantPointerNull` to represent an actual `nullptr` instead of a zero-value pointer #166667

shiltian commented Nov 5, 2025 •

edited

Loading

shiltian Nov 6, 2025 •

edited

Loading

arichardson Nov 7, 2025 •

edited

Loading