-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[AMDGPU][Attributor] Skip update and manifest if an AA is at its initial state #114726
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
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 |
---|---|---|
|
@@ -826,6 +826,16 @@ struct AAAMDSizeRangeAttribute | |
if (!CallerInfo || !CallerInfo->isValidState()) | ||
return false; | ||
|
||
/// When the caller AA is in its initial state, the state remains valid | ||
/// but awaits propagation. We skip processing in this case. Note that we | ||
/// must return true since the state is still considered valid. | ||
if (CallerInfo->isAtInitialState()) { | ||
LLVM_DEBUG(dbgs() << '[' << getName() << "] Caller " | ||
<< Caller->getName() | ||
<< " is still at initial state. Skip the update.\n"); | ||
return true; | ||
} | ||
|
||
Change |= | ||
clampStateAndIndicateChange(this->getState(), CallerInfo->getState()); | ||
|
||
|
@@ -870,6 +880,15 @@ struct AAAMDSizeRangeAttribute | |
/*ForceReplace=*/true); | ||
} | ||
|
||
/// The initial state of `IntegerRangeState` represents an empty set, which | ||
/// does not constitute a valid range. This empty state complicates | ||
/// propagation, particularly for arithmetic operations like | ||
/// `getAssumed().getUpper() - 1`. Therefore, it is recommended to skip the | ||
/// initial state during processing. | ||
bool isAtInitialState() const { | ||
return isValidState() && getAssumed().isEmptySet(); | ||
} | ||
|
||
const std::string getAsStr(Attributor *) const override { | ||
std::string Str; | ||
raw_string_ostream OS(Str); | ||
|
@@ -926,6 +945,11 @@ struct AAAMDFlatWorkGroupSize : public AAAMDSizeRangeAttribute { | |
Attributor &A); | ||
|
||
ChangeStatus manifest(Attributor &A) override { | ||
if (isAtInitialState()) { | ||
LLVM_DEBUG(dbgs() << '[' << getName() | ||
<< "] Still at initial state. No manifest.\n";); | ||
return ChangeStatus::UNCHANGED; | ||
} | ||
Function *F = getAssociatedFunction(); | ||
auto &InfoCache = static_cast<AMDGPUInformationCache &>(A.getInfoCache()); | ||
return emitAttributeIfNotDefaultAfterClamp( | ||
|
@@ -1152,31 +1176,71 @@ struct AAAMDWavesPerEU : public AAAMDSizeRangeAttribute { | |
auto &InfoCache = static_cast<AMDGPUInformationCache &>(A.getInfoCache()); | ||
ChangeStatus Change = ChangeStatus::UNCHANGED; | ||
|
||
Function *F = getAssociatedFunction(); | ||
|
||
const auto *AAFlatWorkGroupSize = A.getAAFor<AAAMDFlatWorkGroupSize>( | ||
*this, IRPosition::function(*F), DepClassTy::REQUIRED); | ||
if (!AAFlatWorkGroupSize || !AAFlatWorkGroupSize->isValidState()) { | ||
LLVM_DEBUG( | ||
dbgs() << '[' << getName() | ||
<< "] AAAMDFlatWorkGroupSize is unavailable or invalid.\n"); | ||
return ChangeStatus::UNCHANGED; | ||
} | ||
|
||
if (AAFlatWorkGroupSize->isAtInitialState()) { | ||
LLVM_DEBUG(dbgs() << '[' << getName() | ||
<< "] AAAMDFlatWorkGroupSize is still at initial " | ||
"state. Skip the update.\n"); | ||
return ChangeStatus::UNCHANGED; | ||
} | ||
Comment on lines
+1190
to
+1195
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. The isAtInitialState check is a superset of the above isValidState, just replace the isValidState call? 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. An initial state is always a valid state, but the reverse is not necessarily true. Once the value is updated, it is no longer in the initial state; however, it remains valid. As such, updates should continue, and the manifest function must still be invoked. 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. There are four states to consider: valid, invalid, best, and worst. An AA typically begins in the best state and transitions toward a "worse" state until reaching the worst state. For most AAs, the invalid state is the worst state, but this is not always true, especially for the In the case of The situation becomes more complex because the best state does not actually represent a valid pair of values in terms of the attribute. However, it must still be considered valid so that updates can occur, allowing Complications arise further because this AA relies on another AA that uses range state. Similarly, the best state of the dependent AA is not a valid pair of values either. If the dependent AA remains in its best (or initial, as introduced in this PR) state, we must skip the update, as its values are effectively nonsensical. |
||
|
||
auto CurrentWorkGroupSize = std::make_pair( | ||
AAFlatWorkGroupSize->getAssumed().getLower().getZExtValue(), | ||
AAFlatWorkGroupSize->getAssumed().getUpper().getZExtValue() - 1); | ||
|
||
auto DoUpdate = [&](std::pair<unsigned, unsigned> WavesPerEU, | ||
std::pair<unsigned, unsigned> FlatWorkGroupSize) { | ||
auto [Min, Max] = | ||
InfoCache.getEffectiveWavesPerEU(*F, WavesPerEU, FlatWorkGroupSize); | ||
ConstantRange CR(APInt(32, Min), APInt(32, Max + 1)); | ||
IntegerRangeState IRS(CR); | ||
Change |= clampStateAndIndicateChange(this->getState(), IRS); | ||
}; | ||
|
||
// We need to clamp once if we are not at initial state, because | ||
// AAAMDFlatWorkGroupSize could be updated in last iteration. | ||
if (!isAtInitialState()) { | ||
auto CurrentWavesPerEU = | ||
std::make_pair(getAssumed().getLower().getZExtValue(), | ||
getAssumed().getUpper().getZExtValue() - 1); | ||
DoUpdate(CurrentWavesPerEU, CurrentWorkGroupSize); | ||
} | ||
|
||
auto CheckCallSite = [&](AbstractCallSite CS) { | ||
Function *Caller = CS.getInstruction()->getFunction(); | ||
Function *Func = getAssociatedFunction(); | ||
|
||
LLVM_DEBUG(dbgs() << '[' << getName() << "] Call " << Caller->getName() | ||
<< "->" << Func->getName() << '\n'); | ||
<< "->" << F->getName() << '\n'); | ||
|
||
const auto *CallerInfo = A.getAAFor<AAAMDWavesPerEU>( | ||
const auto *AAWavesPerEU = A.getAAFor<AAAMDWavesPerEU>( | ||
*this, IRPosition::function(*Caller), DepClassTy::REQUIRED); | ||
const auto *AssumedGroupSize = A.getAAFor<AAAMDFlatWorkGroupSize>( | ||
*this, IRPosition::function(*Func), DepClassTy::REQUIRED); | ||
if (!CallerInfo || !AssumedGroupSize || !CallerInfo->isValidState() || | ||
!AssumedGroupSize->isValidState()) | ||
if (!AAWavesPerEU || !AAWavesPerEU->isValidState()) { | ||
LLVM_DEBUG(dbgs() << '[' << getName() << "] Caller " | ||
<< Caller->getName() | ||
<< " is unavailable or invalid.\n"); | ||
return false; | ||
} | ||
if (AAWavesPerEU->isAtInitialState()) { | ||
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. Same as above |
||
LLVM_DEBUG(dbgs() << '[' << getName() << "] Caller " | ||
<< Caller->getName() | ||
<< " is still at initial state. Skip the update.\n"); | ||
return true; | ||
} | ||
|
||
unsigned Min, Max; | ||
std::tie(Min, Max) = InfoCache.getEffectiveWavesPerEU( | ||
*Caller, | ||
{CallerInfo->getAssumed().getLower().getZExtValue(), | ||
CallerInfo->getAssumed().getUpper().getZExtValue() - 1}, | ||
{AssumedGroupSize->getAssumed().getLower().getZExtValue(), | ||
AssumedGroupSize->getAssumed().getUpper().getZExtValue() - 1}); | ||
ConstantRange CallerRange(APInt(32, Min), APInt(32, Max + 1)); | ||
IntegerRangeState CallerRangeState(CallerRange); | ||
Change |= clampStateAndIndicateChange(this->getState(), CallerRangeState); | ||
|
||
auto CallerWavesPerEU = std::make_pair( | ||
AAWavesPerEU->getAssumed().getLower().getZExtValue(), | ||
AAWavesPerEU->getAssumed().getUpper().getZExtValue() - 1); | ||
DoUpdate(CallerWavesPerEU, CurrentWorkGroupSize); | ||
return true; | ||
}; | ||
|
||
|
@@ -1192,6 +1256,11 @@ struct AAAMDWavesPerEU : public AAAMDSizeRangeAttribute { | |
Attributor &A); | ||
|
||
ChangeStatus manifest(Attributor &A) override { | ||
if (isAtInitialState()) { | ||
LLVM_DEBUG(dbgs() << '[' << getName() | ||
<< "] Still at initial state. No manifest.\n";); | ||
return ChangeStatus::UNCHANGED; | ||
} | ||
Function *F = getAssociatedFunction(); | ||
auto &InfoCache = static_cast<AMDGPUInformationCache &>(A.getInfoCache()); | ||
return emitAttributeIfNotDefaultAfterClamp( | ||
|
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.
use printAsOperand to handle anonymous functions correctly
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.
This is copied from existing code. I'll do a follow up patch to change them all.