-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[Attributor] Propagate alignment through ptrmask #150158
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
Changes from 3 commits
bd9c95b
eecf59e
3a161f3
cd46759
48cd7d6
8091ffb
7b1abf2
461f0b1
258238f
8878983
ec9dccd
420ab45
22ee90f
ebbfc0b
52b16d4
47dc434
4c8af0f
9aa5bdb
9985f51
794c7f0
3d8aaad
50a1bbd
3ee0bba
4b69c00
6241734
4613413
1aaa568
ac0f753
166cbf9
0105ebd
9efeb6f
5ae0e7f
2eeff68
801c6b6
736bd31
63cac5f
2213f73
91bc830
e833e8b
5b95504
a065a18
57b4b2b
5268447
7188f35
09549d9
9755380
2a53aa7
d2445fe
c8ddb15
1dc52e2
bce47af
65e7872
c6c5131
5a9099a
c34df24
9ffe9ef
4fed48b
62277ae
14e6de2
1190cff
db748bf
572e2b2
1432758
edf463a
a4b632b
cb48630
ff78488
62375b2
835bebe
0c5723a
ad59b3e
f930468
add9024
eb3b946
4716ce0
71c1e45
4db0555
7b4737a
7cf4cf5
558acc7
2020b77
15322ab
6937c0a
ad75dcc
c42ed04
91edd4e
fca2371
9fbc571
c5650f1
38cac2f
db99e2e
caf71d2
7ac364f
846e4db
0dc1ac3
37197f2
54e1e6c
8b023c0
bdf2ec6
cf9bc32
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 |
|---|---|---|
|
|
@@ -5203,6 +5203,32 @@ static unsigned getKnownAlignForUse(Attributor &A, AAAlign &QueryingAA, | |
| TrackUse = true; | ||
| return 0; | ||
| } | ||
| if (auto *II = dyn_cast<IntrinsicInst>(I)) { | ||
| if (II->getIntrinsicID() == Intrinsic::ptrmask) { | ||
| auto *ConstVals = A.getAAFor<AAPotentialConstantValues>( | ||
| QueryingAA, IRPosition::value(*(II->getOperand(1))), | ||
| DepClassTy::NONE); | ||
| const AAAlign *AlignAA = A.getAAFor<AAAlign>( | ||
| QueryingAA, IRPosition::value(*(II)), DepClassTy::NONE); | ||
| if (ConstVals && ConstVals->isValidState()) { | ||
| if (ConstVals->isAtFixpoint()) { | ||
| uint64_t TrailingZeros = 64; | ||
| for (const auto &It : ConstVals->getAssumedSet()) | ||
| if (It.countTrailingZeros() < TrailingZeros) | ||
| TrailingZeros = It.countTrailingZeros(); | ||
| if (TrailingZeros < 64) { | ||
|
||
| uint64_t Mask = 1 << TrailingZeros; | ||
| if (Mask >= AlignAA->getKnownAlign().value()) { | ||
| return 0; | ||
| } | ||
| } | ||
| return AlignAA->getKnownAlign().value(); | ||
| } | ||
| } else if (AlignAA) { | ||
| return AlignAA->getKnownAlign().value(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| MaybeAlign MA; | ||
| if (const auto *CB = dyn_cast<CallBase>(I)) { | ||
|
|
@@ -5502,6 +5528,42 @@ struct AAAlignCallSiteReturned final | |
| AAAlignCallSiteReturned(const IRPosition &IRP, Attributor &A) | ||
| : Base(IRP, A) {} | ||
|
|
||
| ChangeStatus updateImpl(Attributor &A) override { | ||
| Instruction *I = getIRPosition().getCtxI(); | ||
shiltian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (auto *II = dyn_cast<IntrinsicInst>(I)) { | ||
| if (II->getIntrinsicID() == Intrinsic::ptrmask) { | ||
| const AAPotentialConstantValues *ConstVals = | ||
| A.getAAFor<AAPotentialConstantValues>( | ||
| *this, IRPosition::value(*(II->getOperand(1))), | ||
| DepClassTy::REQUIRED); | ||
| const AAAlign *AlignAA = | ||
| A.getAAFor<AAAlign>(*this, IRPosition::value(*(II->getOperand(0))), | ||
| DepClassTy::REQUIRED); | ||
| uint64_t Alignment = 0; | ||
| if (ConstVals && ConstVals->isValidState()) { | ||
| unsigned TrailingZeros = 64; | ||
|
||
| for (const auto &It : ConstVals->getAssumedSet()) | ||
| if (It.countTrailingZeros() < TrailingZeros) | ||
| TrailingZeros = It.countTrailingZeros(); | ||
| if (TrailingZeros < 64) | ||
| Alignment = 1 << TrailingZeros; | ||
| } | ||
| if (AlignAA && AlignAA->isValidState() && | ||
| Alignment < AlignAA->getAssumedAlign().value()) | ||
| Alignment = AlignAA->getAssumedAlign().value(); | ||
|
|
||
| if (Alignment != 0) { | ||
| uint64_t OldAssumed = getAssumed(); | ||
| takeAssumedMinimum(Alignment); | ||
| return OldAssumed == getAssumed() ? ChangeStatus::UNCHANGED | ||
| : ChangeStatus::CHANGED; | ||
| } else { | ||
|
||
| return ChangeStatus::UNCHANGED; | ||
| } | ||
| } | ||
| } | ||
| return Base::updateImpl(A); | ||
|
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'm not sure if we want to simply give up here if it is an intrinsic call and 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. Hi @shiltian , same as known alignment I cannot tell from the return value that if it is correctly handled. But what is different here is if it is intrinsic, the call will be equivalently handled as call to deceleration, which will create a pessimistic alignment attribute for the caller. so the following handle can be ignored and directly return here. 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. You can compare with 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. Hi @shiltian , 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'm lost here. 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. Hi @shiltian , 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'm so confused. Inside 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. Hi @shiltian , if the return is However if I'm not wrong, let it continue will simply return the alignment of the intrinsic. Since intrinsics are function declaration, so it will initialize but not update and directly indicate pessimistic and make the "assumed = known". May be we can implement the logic inside 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. When it returns I understand for now it will not make any difference but I'd just like to be a little bit future proof here. |
||
| }; | ||
| /// See AbstractAttribute::trackStatistics() | ||
| void trackStatistics() const override { STATS_DECLTRACK_CS_ATTR(align); } | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,101 @@ | ||||||
| ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 | ||||||
| ; RUN: opt -passes=attributor -S < %s | FileCheck %s | ||||||
|
|
||||||
| define float @align_ptrmask_back_no_prop(ptr align 2 %x, i1 %cmp1, i1 %cmp2) { | ||||||
| ; CHECK-LABEL: define float @align_ptrmask_back_no_prop( | ||||||
| ; CHECK-SAME: ptr nofree readonly align 2 [[X:%.*]], i1 [[CMP1:%.*]], i1 [[CMP2:%.*]]) #[[ATTR0:[0-9]+]] { | ||||||
| ; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP1]], i64 -32, i64 -8 | ||||||
| ; CHECK-NEXT: [[SEL1:%.*]] = select i1 [[CMP2]], i64 [[SEL]], i64 -16 | ||||||
| ; CHECK-NEXT: [[P:%.*]] = tail call align 8 ptr @llvm.ptrmask.p0.i64(ptr [[X]], i64 noundef [[SEL1]]) #[[ATTR2:[0-9]+]] | ||||||
| ; CHECK-NEXT: [[RES:%.*]] = load float, ptr [[P]], align 8, !invariant.load [[META0:![0-9]+]] | ||||||
| ; CHECK-NEXT: ret float [[RES]] | ||||||
| ; | ||||||
| %sel = select i1 %cmp1, i64 -32, i64 -8 | ||||||
| %sel1 = select i1 %cmp2, i64 %sel, i64 -16 | ||||||
| %p = tail call ptr @llvm.ptrmask(ptr %x, i64 %sel1) | ||||||
| %res = load float, ptr %p, align 8 | ||||||
| ret float %res | ||||||
| } | ||||||
|
|
||||||
| define float @align_ptrmask_back_prop(ptr align 2 %x, i1 %cmp1, i1 %cmp2) { | ||||||
| ; CHECK-LABEL: define float @align_ptrmask_back_prop( | ||||||
| ; CHECK-SAME: ptr nofree readonly align 16 [[X:%.*]], i1 [[CMP1:%.*]], i1 [[CMP2:%.*]]) #[[ATTR0]] { | ||||||
| ; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP1]], i64 -32, i64 -8 | ||||||
| ; CHECK-NEXT: [[SEL1:%.*]] = select i1 [[CMP2]], i64 [[SEL]], i64 -16 | ||||||
| ; CHECK-NEXT: [[P:%.*]] = tail call align 16 ptr @llvm.ptrmask.p0.i64(ptr [[X]], i64 noundef [[SEL1]]) #[[ATTR2]] | ||||||
| ; CHECK-NEXT: [[RES:%.*]] = load float, ptr [[P]], align 16, !invariant.load [[META0]] | ||||||
| ; CHECK-NEXT: ret float [[RES]] | ||||||
| ; | ||||||
| %sel = select i1 %cmp1, i64 -32, i64 -8 | ||||||
| %sel1 = select i1 %cmp2, i64 %sel, i64 -16 | ||||||
| %p = tail call ptr @llvm.ptrmask(ptr %x, i64 %sel1) | ||||||
| %res = load float, ptr %p, align 16 | ||||||
| ret float %res | ||||||
| } | ||||||
|
|
||||||
| define float @align_ptrmask_forward_mask(ptr align 2 %x, i1 %cmp1, i1 %cmp2) { | ||||||
| ; CHECK-LABEL: define float @align_ptrmask_forward_mask( | ||||||
| ; CHECK-SAME: ptr nofree readonly align 2 [[X:%.*]], i1 [[CMP1:%.*]], i1 [[CMP2:%.*]]) #[[ATTR0]] { | ||||||
| ; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP1]], i64 -32, i64 -8 | ||||||
| ; CHECK-NEXT: [[SEL1:%.*]] = select i1 [[CMP2]], i64 [[SEL]], i64 -16 | ||||||
| ; CHECK-NEXT: [[P:%.*]] = tail call align 8 ptr @llvm.ptrmask.p0.i64(ptr [[X]], i64 noundef [[SEL1]]) #[[ATTR2]] | ||||||
| ; CHECK-NEXT: [[RES:%.*]] = load float, ptr [[P]], align 8, !invariant.load [[META0]] | ||||||
| ; CHECK-NEXT: ret float [[RES]] | ||||||
| ; | ||||||
| %sel = select i1 %cmp1, i64 -32, i64 -8 | ||||||
| %sel1 = select i1 %cmp2, i64 %sel, i64 -16 | ||||||
| %p = tail call ptr @llvm.ptrmask(ptr %x, i64 %sel1) | ||||||
| %res = load float, ptr %p, align 4 | ||||||
| ret float %res | ||||||
| } | ||||||
|
|
||||||
| define float @align_ptrmask_forward_ptr(ptr align 16 %x, i1 %cmp1, i1 %cmp2) { | ||||||
| ; CHECK-LABEL: define float @align_ptrmask_forward_ptr( | ||||||
| ; CHECK-SAME: ptr nofree readonly align 16 [[X:%.*]], i1 [[CMP1:%.*]], i1 [[CMP2:%.*]]) #[[ATTR0]] { | ||||||
| ; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP1]], i64 -32, i64 -8 | ||||||
| ; CHECK-NEXT: [[SEL1:%.*]] = select i1 [[CMP2]], i64 [[SEL]], i64 -16 | ||||||
| ; CHECK-NEXT: [[P:%.*]] = tail call align 16 ptr @llvm.ptrmask.p0.i64(ptr [[X]], i64 noundef [[SEL1]]) #[[ATTR2]] | ||||||
| ; CHECK-NEXT: [[RES:%.*]] = load float, ptr [[P]], align 16, !invariant.load [[META0]] | ||||||
| ; CHECK-NEXT: ret float [[RES]] | ||||||
| ; | ||||||
| %sel = select i1 %cmp1, i64 -32, i64 -8 | ||||||
| %sel1 = select i1 %cmp2, i64 %sel, i64 -16 | ||||||
| %p = tail call ptr @llvm.ptrmask(ptr %x, i64 %sel1) | ||||||
| %res = load float, ptr %p, align 4 | ||||||
| ret float %res | ||||||
| } | ||||||
|
|
||||||
| define float @align_ptrmask_forward_nonconst_mask(ptr align 8 %x, i64 %y, i1 %cmp1, i1 %cmp2) { | ||||||
| ; CHECK-LABEL: define float @align_ptrmask_forward_nonconst_mask( | ||||||
| ; CHECK-SAME: ptr nofree readonly align 8 [[X:%.*]], i64 [[Y:%.*]], i1 [[CMP1:%.*]], i1 [[CMP2:%.*]]) #[[ATTR0]] { | ||||||
| ; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP1]], i64 -32, i64 [[Y]] | ||||||
| ; CHECK-NEXT: [[SEL1:%.*]] = select i1 [[CMP2]], i64 [[SEL]], i64 -16 | ||||||
| ; CHECK-NEXT: [[P:%.*]] = tail call align 8 ptr @llvm.ptrmask.p0.i64(ptr [[X]], i64 [[SEL1]]) #[[ATTR2]] | ||||||
| ; CHECK-NEXT: [[RES:%.*]] = load float, ptr [[P]], align 8, !invariant.load [[META0]] | ||||||
| ; CHECK-NEXT: ret float [[RES]] | ||||||
| ; | ||||||
| %sel = select i1 %cmp1, i64 -32, i64 %y | ||||||
| %sel1 = select i1 %cmp2, i64 %sel, i64 -16 | ||||||
| %p = tail call ptr @llvm.ptrmask(ptr %x, i64 %sel1) | ||||||
| %res = load float, ptr %p, align 4 | ||||||
| ret float %res | ||||||
|
||||||
| } | ||||||
|
|
||||||
| define float @align_ptrmask_back_nonconst_mask(ptr align 4 %x, i64 %y, i1 %cmp1, i1 %cmp2) { | ||||||
| ; CHECK-LABEL: define float @align_ptrmask_back_nonconst_mask( | ||||||
| ; CHECK-SAME: ptr nofree readonly align 8 [[X:%.*]], i64 [[Y:%.*]], i1 [[CMP1:%.*]], i1 [[CMP2:%.*]]) #[[ATTR0]] { | ||||||
| ; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP1]], i64 -32, i64 [[Y]] | ||||||
| ; CHECK-NEXT: [[SEL1:%.*]] = select i1 [[CMP2]], i64 [[SEL]], i64 -16 | ||||||
| ; CHECK-NEXT: [[P:%.*]] = tail call align 8 ptr @llvm.ptrmask.p0.i64(ptr [[X]], i64 [[SEL1]]) #[[ATTR2]] | ||||||
| ; CHECK-NEXT: [[RES:%.*]] = load float, ptr [[P]], align 8, !invariant.load [[META0]] | ||||||
| ; CHECK-NEXT: ret float [[RES]] | ||||||
| ; | ||||||
| %sel = select i1 %cmp1, i64 -32, i64 %y | ||||||
| %sel1 = select i1 %cmp2, i64 %sel, i64 -16 | ||||||
| %p = tail call ptr @llvm.ptrmask(ptr %x, i64 %sel1) | ||||||
|
||||||
| %p = tail call ptr @llvm.ptrmask(ptr %x, i64 %sel1) | |
| %p = tail call ptr @llvm.ptrmask.p0.i64(ptr %x, i64 %sel1) |
Throughout
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.
Can you add tests with these degenerate cases:
- Input mask value is a small positive constant, such that it cannot increase alignment.
- Mask that exactly matches maximum alignment, i.e. -4294967296
- Mask with 1 more bit than maximum alignment, i.e. -8589934592
- Poison mask
- ptrmask with align attributes on callsite
Outdated
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.
Test cases with constant values, which is the common case. Also test some unknowable cases. Also vectors
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.
Hi @arsenm , I'm not sure if I can find a vector case. The current frame work seems stop it from generating AAAlign attribute for vector case:


Also extractelement cannot propagate alignment, so I also cannot use store/load to test
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.
extractelement can propagate alignment, whether or not it does currently is another question
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.
Hi @arsenm , yep, sorry for how I said it. The current framework seems not supporting propagate alignment through extractelement. Would you like me to add simple support in this PR in order to add vector tests??
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.
No, add the vector test that shows it doesn't propagate for later improvement

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.
Can you split the intrinsic case into a separate function, and switch over the intrinsic ID