Skip to content

Commit 5b4ca85

Browse files
committed
[InferAddressSpaces] Handle unconverted ptrmask
In case a ptrmask cannot be converted to the new address space due to an unknown mask value, this needs to be detcted and an addrspacecast is needed to not hinder a future use of the unconverted return value of ptrmask. Otherwise, users of this value will become invalid by receiving a nullptr as an operand. This LLVM defect was identified via the AMD Fuzzing project.
1 parent bb38b48 commit 5b4ca85

File tree

6 files changed

+368
-63
lines changed

6 files changed

+368
-63
lines changed

llvm/include/llvm/Analysis/TargetTransformInfo.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,18 @@ class TargetTransformInfo {
499499

500500
LLVM_ABI bool isNoopAddrSpaceCast(unsigned FromAS, unsigned ToAS) const;
501501

502+
// Given an address space cast of the given pointer value, calculate the known
503+
// bits of the source pointer in the source addrspace and the destination
504+
// pointer in the destination addrspace.
505+
LLVM_ABI std::pair<KnownBits, KnownBits>
506+
computeKnownBitsAddrSpaceCast(unsigned ToAS, const Value &PtrOp) const;
507+
508+
// Given an address space cast, calculate the known bits of the resulting ptr
509+
// in the destination addrspace using the known bits of the source pointer in
510+
// the source addrspace.
511+
LLVM_ABI KnownBits computeKnownBitsAddrSpaceCast(
512+
unsigned FromAS, unsigned ToAS, const KnownBits &FromPtrBits) const;
513+
502514
/// Return true if globals in this address space can have initializers other
503515
/// than `undef`.
504516
LLVM_ABI bool

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "llvm/Analysis/ScalarEvolutionExpressions.h"
1818
#include "llvm/Analysis/TargetTransformInfo.h"
19+
#include "llvm/Analysis/ValueTracking.h"
1920
#include "llvm/Analysis/VectorUtils.h"
2021
#include "llvm/IR/DataLayout.h"
2122
#include "llvm/IR/GetElementPtrTypeIterator.h"
@@ -151,6 +152,52 @@ class TargetTransformInfoImplBase {
151152
}
152153

153154
virtual bool isNoopAddrSpaceCast(unsigned, unsigned) const { return false; }
155+
156+
virtual std::pair<KnownBits, KnownBits>
157+
computeKnownBitsAddrSpaceCast(unsigned ToAS, const Value &PtrOp) const {
158+
const Type *PtrTy = PtrOp.getType();
159+
assert(PtrTy->isPtrOrPtrVectorTy() &&
160+
"expected pointer or pointer vector type");
161+
unsigned FromAS = PtrTy->getPointerAddressSpace();
162+
163+
if (DL.isNonIntegralAddressSpace(FromAS))
164+
return std::pair(KnownBits(DL.getPointerSizeInBits(FromAS)),
165+
KnownBits(DL.getPointerSizeInBits(ToAS)));
166+
167+
KnownBits FromPtrBits;
168+
if (const AddrSpaceCastInst *CastI = dyn_cast<AddrSpaceCastInst>(&PtrOp)) {
169+
std::pair<KnownBits, KnownBits> KB = computeKnownBitsAddrSpaceCast(
170+
CastI->getDestAddressSpace(), *CastI->getPointerOperand());
171+
FromPtrBits = KB.second;
172+
} else if (FromAS == 0 &&
173+
PatternMatch::match(&PtrOp, PatternMatch::m_Zero())) {
174+
// For addrspace 0, we know that a null pointer has the value 0.
175+
FromPtrBits = KnownBits::makeConstant(
176+
APInt::getZero(DL.getPointerSizeInBits(FromAS)));
177+
} else {
178+
FromPtrBits = computeKnownBits(&PtrOp, DL, nullptr);
179+
}
180+
181+
KnownBits ToPtrBits =
182+
computeKnownBitsAddrSpaceCast(FromAS, ToAS, FromPtrBits);
183+
184+
return std::pair(FromPtrBits, ToPtrBits);
185+
}
186+
187+
virtual KnownBits
188+
computeKnownBitsAddrSpaceCast(unsigned FromAS, unsigned ToAS,
189+
const KnownBits &FromPtrBits) const {
190+
unsigned ToASBitSize = DL.getPointerSizeInBits(ToAS);
191+
192+
if (DL.isNonIntegralAddressSpace(FromAS))
193+
return KnownBits(ToASBitSize);
194+
195+
// By default, we assume that all valid "larger" (e.g. 64-bit) to "smaller"
196+
// (e.g. 32-bit) casts work by chopping off the high bits.
197+
// By default, we do not assume that null results in null again.
198+
return FromPtrBits.anyextOrTrunc(ToASBitSize);
199+
}
200+
154201
virtual bool
155202
canHaveNonUndefGlobalInitializerInAddressSpace(unsigned AS) const {
156203
return AS == 0;

llvm/lib/Analysis/TargetTransformInfo.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,17 @@ bool TargetTransformInfo::isNoopAddrSpaceCast(unsigned FromAS,
330330
return TTIImpl->isNoopAddrSpaceCast(FromAS, ToAS);
331331
}
332332

333+
std::pair<KnownBits, KnownBits>
334+
TargetTransformInfo::computeKnownBitsAddrSpaceCast(unsigned ToAS,
335+
const Value &PtrOp) const {
336+
return TTIImpl->computeKnownBitsAddrSpaceCast(ToAS, PtrOp);
337+
}
338+
339+
KnownBits TargetTransformInfo::computeKnownBitsAddrSpaceCast(
340+
unsigned FromAS, unsigned ToAS, const KnownBits &FromPtrBits) const {
341+
return TTIImpl->computeKnownBitsAddrSpaceCast(FromAS, ToAS, FromPtrBits);
342+
}
343+
333344
bool TargetTransformInfo::canHaveNonUndefGlobalInitializerInAddressSpace(
334345
unsigned AS) const {
335346
return TTIImpl->canHaveNonUndefGlobalInitializerInAddressSpace(AS);

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,41 +1150,6 @@ Value *GCNTTIImpl::rewriteIntrinsicWithAddressSpace(IntrinsicInst *II,
11501150
ConstantInt::getTrue(Ctx) : ConstantInt::getFalse(Ctx);
11511151
return NewVal;
11521152
}
1153-
case Intrinsic::ptrmask: {
1154-
unsigned OldAS = OldV->getType()->getPointerAddressSpace();
1155-
unsigned NewAS = NewV->getType()->getPointerAddressSpace();
1156-
Value *MaskOp = II->getArgOperand(1);
1157-
Type *MaskTy = MaskOp->getType();
1158-
1159-
bool DoTruncate = false;
1160-
1161-
const GCNTargetMachine &TM =
1162-
static_cast<const GCNTargetMachine &>(getTLI()->getTargetMachine());
1163-
if (!TM.isNoopAddrSpaceCast(OldAS, NewAS)) {
1164-
// All valid 64-bit to 32-bit casts work by chopping off the high
1165-
// bits. Any masking only clearing the low bits will also apply in the new
1166-
// address space.
1167-
if (DL.getPointerSizeInBits(OldAS) != 64 ||
1168-
DL.getPointerSizeInBits(NewAS) != 32)
1169-
return nullptr;
1170-
1171-
// TODO: Do we need to thread more context in here?
1172-
KnownBits Known = computeKnownBits(MaskOp, DL, nullptr, II);
1173-
if (Known.countMinLeadingOnes() < 32)
1174-
return nullptr;
1175-
1176-
DoTruncate = true;
1177-
}
1178-
1179-
IRBuilder<> B(II);
1180-
if (DoTruncate) {
1181-
MaskTy = B.getInt32Ty();
1182-
MaskOp = B.CreateTrunc(MaskOp, MaskTy);
1183-
}
1184-
1185-
return B.CreateIntrinsic(Intrinsic::ptrmask, {NewV->getType(), MaskTy},
1186-
{NewV, MaskOp});
1187-
}
11881153
case Intrinsic::amdgcn_flat_atomic_fmax_num:
11891154
case Intrinsic::amdgcn_flat_atomic_fmin_num: {
11901155
Type *DestTy = II->getType();

llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp

Lines changed: 75 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,12 @@ class InferAddressSpacesImpl {
206206

207207
bool isSafeToCastConstAddrSpace(Constant *C, unsigned NewAS) const;
208208

209+
Value *clonePtrMaskWithNewAddressSpace(
210+
IntrinsicInst *I, unsigned NewAddrSpace,
211+
const ValueToValueMapTy &ValueWithNewAddrSpace,
212+
const PredicatedAddrSpaceMapTy &PredicatedAS,
213+
SmallVectorImpl<const Use *> *PoisonUsesToFix) const;
214+
209215
Value *cloneInstructionWithNewAddressSpace(
210216
Instruction *I, unsigned NewAddrSpace,
211217
const ValueToValueMapTy &ValueWithNewAddrSpace,
@@ -651,6 +657,69 @@ static Value *operandWithNewAddressSpaceOrCreatePoison(
651657
return PoisonValue::get(NewPtrTy);
652658
}
653659

660+
// A helper function for cloneInstructionWithNewAddressSpace. Handles the
661+
// conversion of a ptrmask intrinsic instruction.
662+
Value *InferAddressSpacesImpl::clonePtrMaskWithNewAddressSpace(
663+
IntrinsicInst *I, unsigned NewAddrSpace,
664+
const ValueToValueMapTy &ValueWithNewAddrSpace,
665+
const PredicatedAddrSpaceMapTy &PredicatedAS,
666+
SmallVectorImpl<const Use *> *PoisonUsesToFix) const {
667+
const Use &PtrOpUse = I->getArgOperandUse(0);
668+
unsigned OldAddrSpace = PtrOpUse->getType()->getPointerAddressSpace();
669+
Value *MaskOp = I->getArgOperand(1);
670+
Type *MaskTy = MaskOp->getType();
671+
672+
std::optional<KnownBits> OldPtrBits;
673+
std::optional<KnownBits> NewPtrBits;
674+
if (!TTI->isNoopAddrSpaceCast(OldAddrSpace, NewAddrSpace)) {
675+
if (std::optional<std::pair<KnownBits, KnownBits>> KB =
676+
TTI->computeKnownBitsAddrSpaceCast(NewAddrSpace, *PtrOpUse.get())) {
677+
OldPtrBits = KB->first;
678+
NewPtrBits = KB->second;
679+
}
680+
}
681+
682+
// If the pointers in both addrspaces have a bitwise representation and if the
683+
// representation of the new pointer is smaller (fewer bits) than the old one,
684+
// check if the mask is applicable to the ptr in the new addrspace. Any
685+
// masking only clearing the low bits will also apply in the new addrspace
686+
// Note: checking if the mask clears high bits is not sufficient as those
687+
// might have already been 0 in the old ptr.
688+
if (NewPtrBits && OldPtrBits->getBitWidth() > NewPtrBits->getBitWidth()) {
689+
KnownBits MaskBits =
690+
computeKnownBits(MaskOp, *DL, /*AssumptionCache=*/nullptr, I);
691+
// Set all unknown bits of the old ptr to 1, so that we are conservative in
692+
// checking which bits are cleared by the mask.
693+
OldPtrBits->One |= ~OldPtrBits->Zero;
694+
// Check which bits are cleared by the mask in the old ptr.
695+
KnownBits ClearedBits = KnownBits::sub(*OldPtrBits, *OldPtrBits & MaskBits);
696+
697+
// If the mask isn't applicable to the new ptr, leave the ptrmask as-is and
698+
// insert an addrspacecast after it.
699+
if (ClearedBits.countMaxActiveBits() > NewPtrBits->countMaxActiveBits()) {
700+
std::optional<BasicBlock::iterator> InsertPoint =
701+
I->getInsertionPointAfterDef();
702+
assert(InsertPoint && "insertion after ptrmask should be possible");
703+
Type *NewPtrType = getPtrOrVecOfPtrsWithNewAS(I->getType(), NewAddrSpace);
704+
Instruction *AddrSpaceCast =
705+
new AddrSpaceCastInst(I, NewPtrType, "", *InsertPoint);
706+
AddrSpaceCast->setDebugLoc(I->getDebugLoc());
707+
return AddrSpaceCast;
708+
}
709+
}
710+
711+
IRBuilder<> B(I);
712+
if (NewPtrBits) {
713+
MaskTy = MaskTy->getWithNewBitWidth(NewPtrBits->getBitWidth());
714+
MaskOp = B.CreateTrunc(MaskOp, MaskTy);
715+
}
716+
Value *NewPtr = operandWithNewAddressSpaceOrCreatePoison(
717+
PtrOpUse, NewAddrSpace, ValueWithNewAddrSpace, PredicatedAS,
718+
PoisonUsesToFix);
719+
return B.CreateIntrinsic(Intrinsic::ptrmask, {NewPtr->getType(), MaskTy},
720+
{NewPtr, MaskOp});
721+
}
722+
654723
// Returns a clone of `I` with its operands converted to those specified in
655724
// ValueWithNewAddrSpace. Due to potential cycles in the data flow graph, an
656725
// operand whose address space needs to be modified might not exist in
@@ -660,9 +729,6 @@ static Value *operandWithNewAddressSpaceOrCreatePoison(
660729
// Note that we do not necessarily clone `I`, e.g., if it is an addrspacecast
661730
// from a pointer whose type already matches. Therefore, this function returns a
662731
// Value* instead of an Instruction*.
663-
//
664-
// This may also return nullptr in the case the instruction could not be
665-
// rewritten.
666732
Value *InferAddressSpacesImpl::cloneInstructionWithNewAddressSpace(
667733
Instruction *I, unsigned NewAddrSpace,
668734
const ValueToValueMapTy &ValueWithNewAddrSpace,
@@ -683,17 +749,8 @@ Value *InferAddressSpacesImpl::cloneInstructionWithNewAddressSpace(
683749
// Technically the intrinsic ID is a pointer typed argument, so specially
684750
// handle calls early.
685751
assert(II->getIntrinsicID() == Intrinsic::ptrmask);
686-
Value *NewPtr = operandWithNewAddressSpaceOrCreatePoison(
687-
II->getArgOperandUse(0), NewAddrSpace, ValueWithNewAddrSpace,
688-
PredicatedAS, PoisonUsesToFix);
689-
Value *Rewrite =
690-
TTI->rewriteIntrinsicWithAddressSpace(II, II->getArgOperand(0), NewPtr);
691-
if (Rewrite) {
692-
assert(Rewrite != II && "cannot modify this pointer operation in place");
693-
return Rewrite;
694-
}
695-
696-
return nullptr;
752+
return clonePtrMaskWithNewAddressSpace(
753+
II, NewAddrSpace, ValueWithNewAddrSpace, PredicatedAS, PoisonUsesToFix);
697754
}
698755

699756
unsigned AS = TTI->getAssumedAddrSpace(I);
@@ -1331,7 +1388,10 @@ bool InferAddressSpacesImpl::rewriteWithNewAddressSpaces(
13311388

13321389
unsigned OperandNo = PoisonUse->getOperandNo();
13331390
assert(isa<PoisonValue>(NewV->getOperand(OperandNo)));
1334-
NewV->setOperand(OperandNo, ValueWithNewAddrSpace.lookup(PoisonUse->get()));
1391+
WeakTrackingVH NewOp = ValueWithNewAddrSpace.lookup(PoisonUse->get());
1392+
assert(NewOp &&
1393+
"poison replacements in ValueWithNewAddrSpace shouldn't be null");
1394+
NewV->setOperand(OperandNo, NewOp);
13351395
}
13361396

13371397
SmallVector<Instruction *, 16> DeadInstructions;

0 commit comments

Comments
 (0)