-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[WIP][IR][Constants] Change the semantic of ConstantPointerNull to represent an actual nullptr instead of a zero-value pointer
#166667
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| // RUN: %clang_cc1 %s -O0 -triple amdgcn -emit-llvm -o - | FileCheck %s | ||
| // RUN: %clang_cc1 %s -O0 -triple amdgcn---opencl -emit-llvm -o - | FileCheck %s | ||
|
|
||
| // CHECK: target datalayout = "e-m:e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9" | ||
| // CHECK: target datalayout = "e-m:e-p:64:64-p1:64:64-po2:32:32-po3:32:32-p4:64:64-po5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128:128:48-p9:192:256:256:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9" | ||
| void foo(void) {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1497,6 +1497,21 @@ Constant *llvm::ConstantFoldCastOperand(unsigned Opcode, Constant *C, | |
| llvm_unreachable("Missing case"); | ||
| case Instruction::PtrToAddr: | ||
| case Instruction::PtrToInt: | ||
| // If the input is a nullptr, we can fold it to the corresponding nullptr | ||
| // value. | ||
| if (Opcode == Instruction::PtrToInt && C->isNullValue()) { | ||
| if (std::optional<APInt> NullPtrValue = DL.getNullPtrValue( | ||
| C->getType()->getScalarType()->getPointerAddressSpace())) { | ||
| 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"); | ||
| } | ||
| } | ||
| } | ||
| if (auto *CE = dyn_cast<ConstantExpr>(C)) { | ||
| Constant *FoldedValue = nullptr; | ||
| // If the input is an inttoptr, eliminate the pair. This requires knowing | ||
|
|
@@ -1543,6 +1558,13 @@ Constant *llvm::ConstantFoldCastOperand(unsigned Opcode, Constant *C, | |
| } | ||
| break; | ||
| case Instruction::IntToPtr: | ||
| // We can fold it to a null pointer if the input is the nullptr value. | ||
| if (std::optional<APInt> NullPtrValue = DL.getNullPtrValue( | ||
| DestTy->getScalarType()->getPointerAddressSpace())) { | ||
| if ((NullPtrValue->isZero() && C->isZeroValue()) || | ||
| (NullPtrValue->isAllOnes() && C->isAllOnesValue())) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| return Constant::getNullValue(DestTy); | ||
arichardson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| // If the input is a ptrtoint, turn the pair into a ptr to ptr bitcast if | ||
| // the int size is >= the ptr size and the address spaces are the same. | ||
| // This requires knowing the width of a pointer, so it can't be done in | ||
|
|
@@ -1561,6 +1583,24 @@ Constant *llvm::ConstantFoldCastOperand(unsigned Opcode, Constant *C, | |
| } | ||
| } | ||
| break; | ||
| case Instruction::AddrSpaceCast: | ||
| // A null pointer (`ptr addrspace(N) null` in IR presentation, | ||
| // `ConstantPointerNull` in LLVM class, not `nullptr` in C/C++) used to | ||
| // represent a zero-value pointer in the corresponding address space. | ||
| // Therefore, we can't simply fold an address space cast of a null pointer | ||
| // from one address space to another, because on some targets, the nullptr | ||
| // of an address space could be non-zero. | ||
| // | ||
| // Recently, the semantic of `ptr addrspace(N) null` is changed to represent | ||
| // the actual nullptr in the corresponding address space. It can be zero or | ||
| // non-zero, depending on the target. Therefore, we can fold an address | ||
| // space cast of a nullptr from one address space to another. | ||
|
|
||
| // If the input is a nullptr, we can fold it to the corresponding | ||
| // nullptr in the destination address space. | ||
| if (C->isNullValue()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see this property in langref so we should either add it or drop this fold.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is actually the whole point of this PR. The semantic would be, convert the I'll clarify this in the LangRef.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good |
||
| return Constant::getNullValue(DestTy); | ||
| [[fallthrough]]; | ||
| case Instruction::Trunc: | ||
| case Instruction::ZExt: | ||
| case Instruction::SExt: | ||
|
|
@@ -1570,7 +1610,6 @@ Constant *llvm::ConstantFoldCastOperand(unsigned Opcode, Constant *C, | |
| case Instruction::SIToFP: | ||
| case Instruction::FPToUI: | ||
| case Instruction::FPToSI: | ||
| case Instruction::AddrSpaceCast: | ||
| break; | ||
| case Instruction::BitCast: | ||
| return FoldBitCast(C, DestTy, DL); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4275,8 +4275,15 @@ static void emitGlobalConstantImpl(const DataLayout &DL, const Constant *CV, | |||||||||||||||||||||
| return emitGlobalConstantFP(CFP, AP); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (isa<ConstantPointerNull>(CV)) { | ||||||||||||||||||||||
| AP.OutStreamer->emitIntValue(0, Size); | ||||||||||||||||||||||
| if (auto *NullPtr = dyn_cast<ConstantPointerNull>(CV)) { | ||||||||||||||||||||||
| 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); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+4279
to
+4286
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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 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? |
||||||||||||||||||||||
| return; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -126,6 +126,40 @@ Constant *llvm::ConstantFoldCastInstruction(unsigned opc, Constant *V, | |||||||||||||||
| if (isa<PoisonValue>(V)) | ||||||||||||||||
| return PoisonValue::get(DestTy); | ||||||||||||||||
|
|
||||||||||||||||
| if (opc == Instruction::IntToPtr) { | ||||||||||||||||
| // 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; | ||||||||||||||||
|
Comment on lines
+130
to
+134
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah is this because of the |
||||||||||||||||
| // If the input is a ptrtoint(0), we can fold it to the corresponding zero | ||||||||||||||||
| // value pointer. | ||||||||||||||||
| if (auto *CE = dyn_cast<ConstantExpr>(V)) { | ||||||||||||||||
| if (CE->getOpcode() == Instruction::PtrToInt) { | ||||||||||||||||
| if (CE->getOperand(0)->isZeroValue()) | ||||||||||||||||
| return Constant::getZeroValue(DestTy); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| if (opc == Instruction::PtrToInt) { | ||||||||||||||||
| // Similarly, we can't fold ptrtoint(nullptr) to null. | ||||||||||||||||
| if (V->isNullValue()) | ||||||||||||||||
| return nullptr; | ||||||||||||||||
| // If the input is a inttoptr(0), we can fold it to the corresponding | ||||||||||||||||
| // zero value. | ||||||||||||||||
| if (auto *CE = dyn_cast<ConstantExpr>(V)) { | ||||||||||||||||
| if (CE->getOpcode() == Instruction::IntToPtr) { | ||||||||||||||||
| if (CE->getOperand(0)->isZeroValue()) | ||||||||||||||||
| return Constant::getZeroValue(DestTy); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| // However, since the recent change of the semantic of `ptr addrspace(N) | ||||||||||||||||
| // null`, we can fold address space cast of a nullptr to the corresponding | ||||||||||||||||
| // nullptr in the destination address space. | ||||||||||||||||
| if (opc == Instruction::AddrSpaceCast && V->isNullValue()) | ||||||||||||||||
| return Constant::getNullValue(DestTy); | ||||||||||||||||
|
|
||||||||||||||||
| if (isa<UndefValue>(V)) { | ||||||||||||||||
| // zext(undef) = 0, because the top bits will be zero. | ||||||||||||||||
| // sext(undef) = 0, because the top bits will all be the same. | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is equivalent and a bit simpler.