Skip to content

Commit cae33d1

Browse files
nikicnikic
committed
[IR] Require that ptrmask mask matches pointer index size
Currently, the ptrmask intrinsic allows the mask to have any size and will zero-extend or truncate it to the pointer size. However, per the specified semantics, what we would actually need to do is to first zero-extend or truncate it to the pointer index size and then 1-extend it to the pointer size. This seems to leave a lot of room for error, so this patch proposes to make the intrinsic stricter: It now requires that the mask type matches the pointer index type -- a zext or trunc can be done explicitly in IR and should not be part of the intrinsic. Also spell out that the mask is 1-extended to the pointer size if we're talking about the integer representation (this is implied by the GEP expansion).
1 parent f48dab5 commit cae33d1

File tree

14 files changed

+96
-397
lines changed

14 files changed

+96
-397
lines changed

‎llvm/docs/LangRef.rst

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26952,7 +26952,8 @@ Arguments:
2695226952
""""""""""
2695326953

2695426954
The first argument is a pointer or vector of pointers. The second argument is
26955-
an integer or vector of integers.
26955+
an integer or vector of integers with the same bit width as the index type
26956+
size of the first argument.
2695626957

2695726958
Overview:
2695826959
""""""""""
@@ -26965,10 +26966,20 @@ to facilitate alias analysis and underlying-object detection.
2696526966
Semantics:
2696626967
""""""""""
2696726968

26968-
The result of ``ptrmask(ptr, mask)`` is equivalent to
26969-
``getelementptr ptr, (ptrtoint(ptr) & mask) - ptrtoint(ptr)``. Both the returned
26970-
pointer(s) and the first argument are based on the same underlying object (for more
26971-
information on the *based on* terminology see
26969+
The result of ``ptrmask(%ptr, %mask)`` is equivalent to the following expansion,
26970+
where ``iPtrIdx`` is the index type size of the pointer::
26971+
26972+
%intptr = ptrtoint ptr %ptr to iPtrIdx ; this may truncate
26973+
%masked = and iPtrIdx %intptr, %mask
26974+
%diff = sub iPtrIdx %masked, %intptr
26975+
%result = getelementptr i8, ptr %ptr, iPtrIdx %diff
26976+
26977+
Considering this as an operation on the integer representation of the pointer,
26978+
if the pointer index type size is smaller than the pointer type size, this
26979+
implies that the mask is extended with 1 bits to the pointer type size.
26980+
26981+
Both the returned pointer(s) and the first argument are based on the same
26982+
underlying object (for more information on the *based on* terminology see
2697226983
:ref:`the pointer aliasing rules <pointeraliasing>`). If the bitwidth of the
2697326984
mask argument does not match the pointer size of the target, the mask is
2697426985
zero-extended or truncated accordingly.

‎llvm/lib/Analysis/ValueTracking.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1637,8 +1637,8 @@ static void computeKnownBitsFromOperator(const Operator *I,
16371637
const Value *Mask = I->getOperand(1);
16381638
Known2 = KnownBits(Mask->getType()->getScalarSizeInBits());
16391639
computeKnownBits(Mask, Known2, Depth + 1, Q);
1640-
// This is basically a pointer typed and.
1641-
Known &= Known2.zextOrTrunc(Known.getBitWidth());
1640+
// TODO: 1-extend would be more precise.
1641+
Known &= Known2.anyextOrTrunc(BitWidth);
16421642
break;
16431643
}
16441644
case Intrinsic::x86_sse42_crc32_64_64:

‎llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7420,11 +7420,12 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
74207420
}
74217421
case Intrinsic::ptrmask: {
74227422
SDValue Ptr = getValue(I.getOperand(0));
7423-
SDValue Const = getValue(I.getOperand(1));
7423+
SDValue Mask = getValue(I.getOperand(1));
74247424

74257425
EVT PtrVT = Ptr.getValueType();
7426-
setValue(&I, DAG.getNode(ISD::AND, sdl, PtrVT, Ptr,
7427-
DAG.getZExtOrTrunc(Const, sdl, PtrVT)));
7426+
assert(PtrVT == Mask.getValueType() &&
7427+
"Pointers with different index type are not supported by SDAG");
7428+
setValue(&I, DAG.getNode(ISD::AND, sdl, PtrVT, Ptr, Mask));
74287429
return;
74297430
}
74307431
case Intrinsic::threadlocal_address: {

‎llvm/lib/IR/Verifier.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5964,6 +5964,10 @@ void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
59645964
"llvm.ptrmask intrinsic arguments must have the same number of "
59655965
"elements",
59665966
&Call);
5967+
Check(DL.getIndexTypeSizeInBits(Ty0) == Ty1->getScalarSizeInBits(),
5968+
"llvm.ptrmask intrinsic second argument bitwidth must match "
5969+
"pointer index type size of first argument",
5970+
&Call);
59675971
break;
59685972
}
59695973
};

‎llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1966,13 +1966,12 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
19661966
if (match(II->getArgOperand(0),
19671967
m_OneUse(m_Intrinsic<Intrinsic::ptrmask>(m_Value(InnerPtr),
19681968
m_Value(InnerMask))))) {
1969-
if (II->getArgOperand(1)->getType() == InnerMask->getType()) {
1970-
Value *NewMask = Builder.CreateAnd(II->getArgOperand(1), InnerMask);
1971-
return replaceInstUsesWith(
1972-
*II,
1973-
Builder.CreateIntrinsic(InnerPtr->getType(), Intrinsic::ptrmask,
1974-
{InnerPtr, NewMask}));
1975-
}
1969+
assert(II->getArgOperand(1)->getType() == InnerMask->getType() &&
1970+
"Mask types must match");
1971+
Value *NewMask = Builder.CreateAnd(II->getArgOperand(1), InnerMask);
1972+
return replaceInstUsesWith(
1973+
*II, Builder.CreateIntrinsic(InnerPtr->getType(), Intrinsic::ptrmask,
1974+
{InnerPtr, NewMask}));
19761975
}
19771976
break;
19781977
}

‎llvm/test/CodeGen/AArch64/lower-ptrmask.ll

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,3 @@ define ptr @test1(ptr %src) {
1212
%ptr = call ptr @llvm.ptrmask.p0.i64(ptr %src, i64 72057594037927928)
1313
ret ptr %ptr
1414
}
15-
16-
declare ptr @llvm.ptrmask.p0.i32(ptr, i32)
17-
18-
; CHECK-LABEL: name: test2
19-
; CHECK: %0:gpr64 = COPY $x0
20-
; CHECK-NEXT: %1:gpr32 = MOVi32imm 10000
21-
; CHECK-NEXT: %2:gpr64 = SUBREG_TO_REG 0, killed %1, %subreg.sub_32
22-
; CHECK-NEXT: %3:gpr64 = ANDXrr %0, killed %2
23-
; CHECK-NEXT: $x0 = COPY %3
24-
; CHECK-NEXT: RET_ReallyLR implicit $x0
25-
26-
define ptr @test2(ptr %src) {
27-
%ptr = call ptr @llvm.ptrmask.p0.i32(ptr %src, i32 10000)
28-
ret ptr %ptr
29-
}

‎llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-ptrmask.ll

Lines changed: 0 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -21,78 +21,6 @@ define ptr @ptrmask_flat_i64(ptr %ptr, i64 %mask) {
2121
ret ptr %masked
2222
}
2323

24-
define ptr @ptrmask_flat_i32(ptr %ptr, i32 %mask) {
25-
; CHECK-LABEL: name: ptrmask_flat_i32
26-
; CHECK: bb.1 (%ir-block.0):
27-
; CHECK-NEXT: liveins: $vgpr0, $vgpr1, $vgpr2
28-
; CHECK-NEXT: {{ $}}
29-
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
30-
; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
31-
; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)
32-
; CHECK-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $vgpr2
33-
; CHECK-NEXT: [[PTRMASK:%[0-9]+]]:_(p0) = G_PTRMASK [[MV]], [[COPY2]](s32)
34-
; CHECK-NEXT: [[UV:%[0-9]+]]:_(s32), [[UV1:%[0-9]+]]:_(s32) = G_UNMERGE_VALUES [[PTRMASK]](p0)
35-
; CHECK-NEXT: $vgpr0 = COPY [[UV]](s32)
36-
; CHECK-NEXT: $vgpr1 = COPY [[UV1]](s32)
37-
; CHECK-NEXT: SI_RETURN implicit $vgpr0, implicit $vgpr1
38-
%masked = call ptr @llvm.ptrmask.p0.i32(ptr %ptr, i32 %mask)
39-
ret ptr %masked
40-
}
41-
42-
define ptr @ptrmask_flat_i16(ptr %ptr, i16 %mask) {
43-
; CHECK-LABEL: name: ptrmask_flat_i16
44-
; CHECK: bb.1 (%ir-block.0):
45-
; CHECK-NEXT: liveins: $vgpr0, $vgpr1, $vgpr2
46-
; CHECK-NEXT: {{ $}}
47-
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
48-
; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
49-
; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)
50-
; CHECK-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $vgpr2
51-
; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s16) = G_TRUNC [[COPY2]](s32)
52-
; CHECK-NEXT: [[PTRMASK:%[0-9]+]]:_(p0) = G_PTRMASK [[MV]], [[TRUNC]](s16)
53-
; CHECK-NEXT: [[UV:%[0-9]+]]:_(s32), [[UV1:%[0-9]+]]:_(s32) = G_UNMERGE_VALUES [[PTRMASK]](p0)
54-
; CHECK-NEXT: $vgpr0 = COPY [[UV]](s32)
55-
; CHECK-NEXT: $vgpr1 = COPY [[UV1]](s32)
56-
; CHECK-NEXT: SI_RETURN implicit $vgpr0, implicit $vgpr1
57-
%masked = call ptr @llvm.ptrmask.p0.i16(ptr %ptr, i16 %mask)
58-
ret ptr %masked
59-
}
60-
61-
define ptr @ptrmask_flat_i1(ptr %ptr, i1 %mask) {
62-
; CHECK-LABEL: name: ptrmask_flat_i1
63-
; CHECK: bb.1 (%ir-block.0):
64-
; CHECK-NEXT: liveins: $vgpr0, $vgpr1, $vgpr2
65-
; CHECK-NEXT: {{ $}}
66-
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
67-
; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
68-
; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)
69-
; CHECK-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $vgpr2
70-
; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s1) = G_TRUNC [[COPY2]](s32)
71-
; CHECK-NEXT: [[PTRMASK:%[0-9]+]]:_(p0) = G_PTRMASK [[MV]], [[TRUNC]](s1)
72-
; CHECK-NEXT: [[UV:%[0-9]+]]:_(s32), [[UV1:%[0-9]+]]:_(s32) = G_UNMERGE_VALUES [[PTRMASK]](p0)
73-
; CHECK-NEXT: $vgpr0 = COPY [[UV]](s32)
74-
; CHECK-NEXT: $vgpr1 = COPY [[UV1]](s32)
75-
; CHECK-NEXT: SI_RETURN implicit $vgpr0, implicit $vgpr1
76-
%masked = call ptr @llvm.ptrmask.p0.i1(ptr %ptr, i1 %mask)
77-
ret ptr %masked
78-
}
79-
80-
define ptr addrspace(3) @ptrmask_local_i64(ptr addrspace(3) %ptr, i64 %mask) {
81-
; CHECK-LABEL: name: ptrmask_local_i64
82-
; CHECK: bb.1 (%ir-block.0):
83-
; CHECK-NEXT: liveins: $vgpr0, $vgpr1, $vgpr2
84-
; CHECK-NEXT: {{ $}}
85-
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(p3) = COPY $vgpr0
86-
; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
87-
; CHECK-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $vgpr2
88-
; CHECK-NEXT: [[MV:%[0-9]+]]:_(s64) = G_MERGE_VALUES [[COPY1]](s32), [[COPY2]](s32)
89-
; CHECK-NEXT: [[PTRMASK:%[0-9]+]]:_(p3) = G_PTRMASK [[COPY]], [[MV]](s64)
90-
; CHECK-NEXT: $vgpr0 = COPY [[PTRMASK]](p3)
91-
; CHECK-NEXT: SI_RETURN implicit $vgpr0
92-
%masked = call ptr addrspace(3) @llvm.ptrmask.p3.i64(ptr addrspace(3) %ptr, i64 %mask)
93-
ret ptr addrspace(3) %masked
94-
}
95-
9624
define ptr addrspace(3) @ptrmask_local_i32(ptr addrspace(3) %ptr, i32 %mask) {
9725
; CHECK-LABEL: name: ptrmask_local_i32
9826
; CHECK: bb.1 (%ir-block.0):
@@ -107,47 +35,11 @@ define ptr addrspace(3) @ptrmask_local_i32(ptr addrspace(3) %ptr, i32 %mask) {
10735
ret ptr addrspace(3) %masked
10836
}
10937

110-
define ptr addrspace(3) @ptrmask_local_i16(ptr addrspace(3) %ptr, i16 %mask) {
111-
; CHECK-LABEL: name: ptrmask_local_i16
112-
; CHECK: bb.1 (%ir-block.0):
113-
; CHECK-NEXT: liveins: $vgpr0, $vgpr1
114-
; CHECK-NEXT: {{ $}}
115-
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(p3) = COPY $vgpr0
116-
; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
117-
; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s16) = G_TRUNC [[COPY1]](s32)
118-
; CHECK-NEXT: [[PTRMASK:%[0-9]+]]:_(p3) = G_PTRMASK [[COPY]], [[TRUNC]](s16)
119-
; CHECK-NEXT: $vgpr0 = COPY [[PTRMASK]](p3)
120-
; CHECK-NEXT: SI_RETURN implicit $vgpr0
121-
%masked = call ptr addrspace(3) @llvm.ptrmask.p3.i16(ptr addrspace(3) %ptr, i16 %mask)
122-
ret ptr addrspace(3) %masked
123-
}
124-
125-
define ptr addrspace(3) @ptrmask_local_i1(ptr addrspace(3) %ptr, i1 %mask) {
126-
; CHECK-LABEL: name: ptrmask_local_i1
127-
; CHECK: bb.1 (%ir-block.0):
128-
; CHECK-NEXT: liveins: $vgpr0, $vgpr1
129-
; CHECK-NEXT: {{ $}}
130-
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(p3) = COPY $vgpr0
131-
; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
132-
; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s1) = G_TRUNC [[COPY1]](s32)
133-
; CHECK-NEXT: [[PTRMASK:%[0-9]+]]:_(p3) = G_PTRMASK [[COPY]], [[TRUNC]](s1)
134-
; CHECK-NEXT: $vgpr0 = COPY [[PTRMASK]](p3)
135-
; CHECK-NEXT: SI_RETURN implicit $vgpr0
136-
%masked = call ptr addrspace(3) @llvm.ptrmask.p3.i1(ptr addrspace(3) %ptr, i1 %mask)
137-
ret ptr addrspace(3) %masked
138-
}
139-
14038
; Seems to not work
14139
; define <2 x ptr> @ptrmask_flat_i64_v2(<2 x ptr> %ptr, <2 x i64> %mask) {
14240
; %masked = call <2 x ptr> @llvm.ptrmask.v2p0.v2i64(<2 x ptr> %ptr, <2 x i64> %mask)
14341
; ret <2 x ptr> %masked
14442
; }
14543

14644
declare ptr @llvm.ptrmask.p0.i64(ptr, i64)
147-
declare ptr @llvm.ptrmask.p0.i32(ptr, i32)
148-
declare ptr @llvm.ptrmask.p0.i16(ptr, i16)
149-
declare ptr @llvm.ptrmask.p0.i1(ptr, i1)
150-
declare ptr addrspace(3) @llvm.ptrmask.p3.i64(ptr addrspace(3), i64)
15145
declare ptr addrspace(3) @llvm.ptrmask.p3.i32(ptr addrspace(3), i32)
152-
declare ptr addrspace(3) @llvm.ptrmask.p3.i16(ptr addrspace(3), i16)
153-
declare ptr addrspace(3) @llvm.ptrmask.p3.i1(ptr addrspace(3), i1)

0 commit comments

Comments
 (0)