-
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 all 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 |
|---|---|---|
|
|
@@ -5185,6 +5185,7 @@ struct AADereferenceableCallSiteReturned final | |
| // ------------------------ Align Argument Attribute ------------------------ | ||
|
|
||
| namespace { | ||
|
|
||
| static unsigned getKnownAlignForUse(Attributor &A, AAAlign &QueryingAA, | ||
| Value &AssociatedValue, const Use *U, | ||
| const Instruction *I, bool &TrackUse) { | ||
|
|
@@ -5200,6 +5201,28 @@ static unsigned getKnownAlignForUse(Attributor &A, AAAlign &QueryingAA, | |
| TrackUse = true; | ||
| return 0; | ||
| } | ||
| if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) | ||
| switch (II->getIntrinsicID()) { | ||
| case Intrinsic::ptrmask: { | ||
| // Is it appropriate to pull attribute in initialization? | ||
| const auto *ConstVals = A.getAAFor<AAPotentialConstantValues>( | ||
| QueryingAA, IRPosition::value(*II->getOperand(1)), DepClassTy::NONE); | ||
| const auto *AlignAA = A.getAAFor<AAAlign>( | ||
| QueryingAA, IRPosition::value(*II), DepClassTy::NONE); | ||
| if (ConstVals && ConstVals->isValidState() && ConstVals->isAtFixpoint()) { | ||
| unsigned ShiftValue = std::min(ConstVals->getAssumedMinTrailingZeros(), | ||
| Value::MaxAlignmentExponent); | ||
| Align ConstAlign(UINT64_C(1) << ShiftValue); | ||
| if (ConstAlign >= AlignAA->getKnownAlign()) | ||
| return Align(1).value(); | ||
| } | ||
| if (AlignAA) | ||
| return AlignAA->getKnownAlign().value(); | ||
| break; | ||
| } | ||
| default: | ||
| break; | ||
| } | ||
|
|
||
| MaybeAlign MA; | ||
| if (const auto *CB = dyn_cast<CallBase>(I)) { | ||
|
|
@@ -5499,6 +5522,44 @@ 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 (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) { | ||
| switch (II->getIntrinsicID()) { | ||
| case Intrinsic::ptrmask: { | ||
| Align Alignment; | ||
| bool Valid = false; | ||
|
|
||
| const auto *ConstVals = A.getAAFor<AAPotentialConstantValues>( | ||
| *this, IRPosition::value(*II->getOperand(1)), DepClassTy::REQUIRED); | ||
| if (ConstVals && ConstVals->isValidState()) { | ||
| unsigned ShiftValue = | ||
| std::min(ConstVals->getAssumedMinTrailingZeros(), | ||
| Value::MaxAlignmentExponent); | ||
| Alignment = Align(UINT64_C(1) << ShiftValue); | ||
| Valid = true; | ||
| } | ||
|
|
||
| const auto *AlignAA = | ||
| A.getAAFor<AAAlign>(*this, IRPosition::value(*(II->getOperand(0))), | ||
| DepClassTy::REQUIRED); | ||
| if (AlignAA && AlignAA->isValidState()) { | ||
| Alignment = std::max(AlignAA->getAssumedAlign(), Alignment); | ||
| Valid = true; | ||
| } | ||
|
|
||
| if (Valid) | ||
| return clampStateAndIndicateChange<StateType>( | ||
| this->getState(), | ||
| std::min(this->getAssumedAlign(), Alignment).value()); | ||
| break; | ||
| } | ||
| default: | ||
| break; | ||
| } | ||
| } | ||
| 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); } | ||
| }; | ||
|
|
||

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 might just not be understanding the attributor very well, but it seems weird that we're setting align 1 here. I assume it's because there's some handling later that fixes this case?
I can tell from the tests that all this code is doing what I'd intuitively expect, so I'll just let myself be confused.
Uh oh!
There was an error while loading. Please reload this page.
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 @krzysz00 this means the mask is going to "cancel out" the alignment requirement, say if we have:
And we are calculating the alignment of
%x, since%pis masked by-16, it is aligned at 16. Then it doesn't matter what is the alignment of%x. So this use of%xreturns 1, which means no alignment requirement of this use for%x. The final required alignment of%xdepends on all of its use.Otherwise, if we have:
Then if
%xis aligned at 4, it must not meet the requirement for thestoreinstruction, so the alignment of%xshould be at least 8. Same, for this use of%xit requires%x's alignment is at least 8