Please use GitHub pull requests for new patches. Phabricator shutdown timeline

Differential D109468 Diff 374440 llvm/lib/Transforms/IPO/OpenMPOpt.cpp

Changeset View

Standalone View

View Options

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

Show First 20 Lines • Show All 573 Lines • ▼ Show 20 Lines struct KernelInfoState : AbstractState {

bool isValidState() const override { return true; } bool isValidState() const override { return true; }

/// See AbstractState::isAtFixpoint(...) /// See AbstractState::isAtFixpoint(...)

bool isAtFixpoint() const override { return IsAtFixpoint; } bool isAtFixpoint() const override { return IsAtFixpoint; }

/// See AbstractState::indicatePessimisticFixpoint(...) /// See AbstractState::indicatePessimisticFixpoint(...)

ChangeStatus indicatePessimisticFixpoint() override { ChangeStatus indicatePessimisticFixpoint() override {

IsAtFixpoint = true; IsAtFixpoint = true;

ReachingKernelEntries.indicatePessimisticFixpoint();

SPMDCompatibilityTracker.indicatePessimisticFixpoint(); SPMDCompatibilityTracker.indicatePessimisticFixpoint();

ReachedKnownParallelRegions.indicatePessimisticFixpoint();

ReachedUnknownParallelRegions.indicatePessimisticFixpoint(); ReachedUnknownParallelRegions.indicatePessimisticFixpoint();

return ChangeStatus::CHANGED; return ChangeStatus::CHANGED;

} }

/// See AbstractState::indicateOptimisticFixpoint(...) /// See AbstractState::indicateOptimisticFixpoint(...)

ChangeStatus indicateOptimisticFixpoint() override { ChangeStatus indicateOptimisticFixpoint() override {

IsAtFixpoint = true; IsAtFixpoint = true;

return ChangeStatus::UNCHANGED; return ChangeStatus::UNCHANGED;

Show All 30 Lines struct KernelInfoState : AbstractState {

/// Return full set as the worst state of potential values. /// Return full set as the worst state of potential values.

static KernelInfoState getWorstState() { return KernelInfoState(false); } static KernelInfoState getWorstState() { return KernelInfoState(false); }

/// "Clamp" this state with \p KIS. /// "Clamp" this state with \p KIS.

KernelInfoState operator^=(const KernelInfoState &KIS) { KernelInfoState operator^=(const KernelInfoState &KIS) {

// Do not merge two different _init and _deinit call sites. // Do not merge two different _init and _deinit call sites.

if (KIS.KernelInitCB) { if (KIS.KernelInitCB) {

if (KernelInitCB && KernelInitCB != KIS.KernelInitCB) if(KernelInitCB && KernelInitCB != KIS.KernelInitCB)

indicatePessimisticFixpoint(); llvm_unreachable("Kernel that calls another kernel violates OpenMP-Opt assumptions.");

tianshilei1992Unsubmitted

Not Done

if (KIS.KernelInitCB) {

- if(KernelInitCB && KernelInitCB != KIS.KernelInitCB)

+ if (KernelInitCB && KernelInitCB != KIS.KernelInitCB)

llvm_unreachable("Kernel that calls another kernel violates OpenMP-Opt assumptions.");

format

tianshilei1992: format

KernelInitCB = KIS.KernelInitCB; KernelInitCB = KIS.KernelInitCB;

} }

if (KIS.KernelDeinitCB) { if (KIS.KernelDeinitCB) {

if (KernelDeinitCB && KernelDeinitCB != KIS.KernelDeinitCB) if(KernelDeinitCB && KernelDeinitCB != KIS.KernelDeinitCB)

indicatePessimisticFixpoint(); llvm_unreachable("Kernel that calls another kernel violates OpenMP-Opt assumptions.");

KernelDeinitCB = KIS.KernelDeinitCB; KernelDeinitCB = KIS.KernelDeinitCB;

} }

SPMDCompatibilityTracker ^= KIS.SPMDCompatibilityTracker; SPMDCompatibilityTracker ^= KIS.SPMDCompatibilityTracker;

ReachedKnownParallelRegions ^= KIS.ReachedKnownParallelRegions; ReachedKnownParallelRegions ^= KIS.ReachedKnownParallelRegions;

ReachedUnknownParallelRegions ^= KIS.ReachedUnknownParallelRegions; ReachedUnknownParallelRegions ^= KIS.ReachedUnknownParallelRegions;

return *this; return *this;

} }

▲ Show 20 Lines • Show All 2,179 Lines • ▼ Show 20 Lines struct AAKernelInfo : public StateWrapper<KernelInfoState, AbstractAttribute> {

const std::string getAsStr() const override { const std::string getAsStr() const override {

if (!isValidState()) if (!isValidState())

return "<invalid>"; return "<invalid>";

return std::string(SPMDCompatibilityTracker.isAssumed() ? "SPMD" return std::string(SPMDCompatibilityTracker.isAssumed() ? "SPMD"

: "generic") + : "generic") +

std::string(SPMDCompatibilityTracker.isAtFixpoint() ? " [FIX]" std::string(SPMDCompatibilityTracker.isAtFixpoint() ? " [FIX]"

: "") + : "") +

std::string(" #PRs: ") + std::string(" #PRs: ") +

std::to_string(ReachedKnownParallelRegions.size()) + (ReachedKnownParallelRegions.isValidState()

? std::to_string(ReachedKnownParallelRegions.size())

: "<invalid>") +

", #Unknown PRs: " + ", #Unknown PRs: " +

std::to_string(ReachedUnknownParallelRegions.size()) + (ReachedUnknownParallelRegions.isValidState()

? std::to_string(ReachedUnknownParallelRegions.size())

: "<invalid>") +

", #Reaching Kernels: " + ", #Reaching Kernels: " +

(ReachingKernelEntries.isValidState() (ReachingKernelEntries.isValidState()

? std::to_string(ReachingKernelEntries.size()) ? std::to_string(ReachingKernelEntries.size())

: "<invalid>"); : "<invalid>");

} }

/// Create an abstract attribute biew for the position \p IRP. /// Create an abstract attribute biew for the position \p IRP.

static AAKernelInfo &createForPosition(const IRPosition &IRP, Attributor &A); static AAKernelInfo &createForPosition(const IRPosition &IRP, Attributor &A);

▲ Show 20 Lines • Show All 189 Lines • ▼ Show 20 Lines ChangeStatus manifest(Attributor &A) override {

// Known SPMD-mode kernels need no manifest changes. // Known SPMD-mode kernels need no manifest changes.

if (SPMDCompatibilityTracker.isKnown()) if (SPMDCompatibilityTracker.isKnown())

return ChangeStatus::UNCHANGED; return ChangeStatus::UNCHANGED;

// If we can we change the execution mode to SPMD-mode otherwise we build a // If we can we change the execution mode to SPMD-mode otherwise we build a

// custom state machine. // custom state machine.

if (!mayContainParallelRegion() || !changeToSPMDMode(A)) if (!mayContainParallelRegion() || !changeToSPMDMode(A))

buildCustomStateMachine(A); return buildCustomStateMachine(A);

tianshilei1992Unsubmitted

Not Done

Don't you expect anything else between in the future?

tianshilei1992: Don't you expect anything else between in the future?

jdoerfertAuthorUnsubmitted

Done

I do not understand the question. Generally, if this is not proper in the future we can change it.
For now, we should return the right thing, that is unchanged if we did not do anything.

jdoerfert: I do not understand the question. Generally, if this is not proper in the future we can change…

tianshilei1992Unsubmitted

Not Done

If we don't expect anything (any other manipulation) in the future between line 3006 and 3008, the change here is fine.

tianshilei1992: If we don't expect anything (any other manipulation) in the future between line 3006 and 3008…

return ChangeStatus::CHANGED; return ChangeStatus::CHANGED;

} }

bool changeToSPMDMode(Attributor &A) { bool changeToSPMDMode(Attributor &A) {

auto &OMPInfoCache = static_cast<OMPInformationCache &>(A.getInfoCache()); auto &OMPInfoCache = static_cast<OMPInformationCache &>(A.getInfoCache());

if (!SPMDCompatibilityTracker.isAssumed()) { if (!SPMDCompatibilityTracker.isAssumed()) {

▲ Show 20 Lines • Show All 287 Lines • ▼ Show 20 Lines bool changeToSPMDMode(Attributor &A) {

}; };

A.emitRemark<OptimizationRemark>(KernelInitCB, "OMP120", Remark); A.emitRemark<OptimizationRemark>(KernelInitCB, "OMP120", Remark);

return true; return true;

}; };

ChangeStatus buildCustomStateMachine(Attributor &A) { ChangeStatus buildCustomStateMachine(Attributor &A) {

// If we have disabled state machine rewrites, don't make a custom one // If we have disabled state machine rewrites, don't make a custom one

if (DisableOpenMPOptStateMachineRewrite) if (DisableOpenMPOptStateMachineRewrite)

return indicatePessimisticFixpoint(); return ChangeStatus::UNCHANGED;

tianshilei1992Unsubmitted

Not Done

From Attributor's perspective, they behave same, don't they?

tianshilei1992: From `Attributor`'s perspective, they behave same, don't they?

jdoerfertAuthorUnsubmitted

Done

Not if we look at AAKernelInfo as part of the manifest of another AA, which we might already do.
So folding might not be performed because AAKernelInfo is invalid while we only wanted to disable the state machine customization.

jdoerfert: Not if we look at AAKernelInfo as part of the manifest of another AA, which we might already do.

assert(ReachedKnownParallelRegions.isValidState() && assert(ReachedKnownParallelRegions.isValidState() &&

"Custom state machine with invalid parallel region states?"); "Custom state machine with invalid parallel region states?");

const int InitModeArgNo = 1; const int InitModeArgNo = 1;

const int InitUseStateMachineArgNo = 2; const int InitUseStateMachineArgNo = 2;

// Check if the current configuration is non-SPMD and generic state machine. // Check if the current configuration is non-SPMD and generic state machine.

▲ Show 20 Lines • Show All 326 Lines • ▼ Show 20 Lines auto CheckCallInst = [&](Instruction &I) {

*this, IRPosition::callsite_function(CB), DepClassTy::OPTIONAL); *this, IRPosition::callsite_function(CB), DepClassTy::OPTIONAL);

getState() ^= CBAA.getState(); getState() ^= CBAA.getState();

AllSPMDStatesWereFixed &= CBAA.SPMDCompatibilityTracker.isAtFixpoint(); AllSPMDStatesWereFixed &= CBAA.SPMDCompatibilityTracker.isAtFixpoint();

return true; return true;

}; };

bool UsedAssumedInformationInCheckCallInst = false; bool UsedAssumedInformationInCheckCallInst = false;

if (!A.checkForAllCallLikeInstructions( if (!A.checkForAllCallLikeInstructions(

CheckCallInst, *this, UsedAssumedInformationInCheckCallInst)) CheckCallInst, *this, UsedAssumedInformationInCheckCallInst)) {

LLVM_DEBUG(dbgs() << TAG << "Failed to visit all call-like instructions!\n";);

return indicatePessimisticFixpoint(); return indicatePessimisticFixpoint();

}

// If we haven't used any assumed information for the SPMD state we can fix // If we haven't used any assumed information for the SPMD state we can fix

// it. // it.

if (!UsedAssumedInformationInCheckRWInst && if (!UsedAssumedInformationInCheckRWInst &&

!UsedAssumedInformationInCheckCallInst && AllSPMDStatesWereFixed) !UsedAssumedInformationInCheckCallInst && AllSPMDStatesWereFixed)

SPMDCompatibilityTracker.indicateOptimisticFixpoint(); SPMDCompatibilityTracker.indicateOptimisticFixpoint();

return StateBefore == getState() ? ChangeStatus::UNCHANGED return StateBefore == getState() ? ChangeStatus::UNCHANGED

▲ Show 20 Lines • Show All 190 Lines • ▼ Show 20 Lines case OMPRTL___kmpc_parallel_51:

// The condition above should usually get the parallel region function // The condition above should usually get the parallel region function

// pointer and record it. In the off chance it doesn't we assume the // pointer and record it. In the off chance it doesn't we assume the

// worst. // worst.

ReachedUnknownParallelRegions.insert(&CB); ReachedUnknownParallelRegions.insert(&CB);

break; break;

case OMPRTL___kmpc_omp_task: case OMPRTL___kmpc_omp_task:

// We do not look into tasks right now, just give up. // We do not look into tasks right now, just give up.

SPMDCompatibilityTracker.insert(&CB); SPMDCompatibilityTracker.insert(&CB);

ReachedUnknownParallelRegions.insert(&CB); ReachedUnknownParallelRegions.insert(&CB);

indicatePessimisticFixpoint(); break;

tianshilei1992Unsubmitted

Not Done

IIRC, ReachedUnknownParallelRegions will be invalidated on insertion. With this change, this AA indicates optimistic fixed point?

tianshilei1992: IIRC, `ReachedUnknownParallelRegions` will be invalidated on insertion. With this change, this…

jdoerfertAuthorUnsubmitted

Done

Yes. But optimistic fixpoint != everything is optimistic. It will move the known state to the assumed state. Given that we already invalidated ReachedUnknownParallelRegions before, it won't "un-invalidate" it, ever.

jdoerfert: Yes. But optimistic fixpoint != everything is optimistic. It will move the known state to the…

tianshilei1992Unsubmitted

Done

I'm fine as long as this is consistent. But in other hand, I feel it might be better to invalid the AA as well if it is using another state that has already been invalidated.

tianshilei1992: I'm fine as long as this is consistent. But in other hand, I feel it might be better to invalid…

jdoerfertAuthorUnsubmitted

Done

I don't follow. If an AA (X) uses another one (Y) and Y has an invalid state, X should not use the state of Y, ever. That is not a question nor is it changed here in any way. What is changed here is the fact that we do not need to give up on all information provided by AAKernelInfoCallSite if we see an __kmpc_omp_task call. We cannot argue SPMD and we cannot know all unknown parallel regions but we can certainly still know ReachingKernels (among other things). Indicating a pessimistic fixpoint here will give up on all information including stuff that we can use without problem.

jdoerfert: I don't follow. If an AA (X) uses another one (Y) and Y has an invalid state, X should not use…

tianshilei1992Unsubmitted

Not Done

indicatePessimisticFixpoint invalidates the AA (X), which is AAKernelInfoCallSite here, as well. When another AA (Y) uses its member, such as ReachingKernels, if we only check the state of Y, it's gonna break. If we directly check the member instead, then we are good. That's what I mean by "consistent".

But yeah, I agree now that we could use partial information. However, if it is fixed point, no matter optimistic or pessimistic, it will not be updated, right? What if the partial information we want to use gets updated? Is it possible?

tianshilei1992: `indicatePessimisticFixpoint` invalidates the AA (X), which is `AAKernelInfoCallSite` here, as…

jdoerfertAuthorUnsubmitted

Done

A pessimistic fixpoint on AAKernelInfo should (almost) never be necessary. This patch removes (almost) all occurrences. A pessimistic fixpoint on AAKernelInfo will invalidate all information in the state, basically reverting everything to the previously known state. Since we track various things deduced in different ways, there is basically no good reason to invalidate everything at once. One could argue we should not track everything in a single state but that is a different discussion.

In your example, if X is invalidated it also invalidates reaching kernels. So Y will realize reaching kernels of X are unusable and deal with it accordingly (probably give up). However, the fact that this is a task runtime call did not impact the reaching kernels so there was no reason to invalidate it in the first place.

jdoerfert: A pessimistic fixpoint on AAKernelInfo should (almost) never be necessary. This patch removes…

return;

case OMPRTL___kmpc_alloc_shared: case OMPRTL___kmpc_alloc_shared:

case OMPRTL___kmpc_free_shared: case OMPRTL___kmpc_free_shared:

// Return without setting a fixpoint, to be resolved in updateImpl. // Return without setting a fixpoint, to be resolved in updateImpl.

return; return;

default: default:

// Unknown OpenMP runtime calls cannot be executed in SPMD-mode, // Unknown OpenMP runtime calls cannot be executed in SPMD-mode,

// generally. // generally. However, they do not hide parallel regions.

SPMDCompatibilityTracker.insert(&CB); SPMDCompatibilityTracker.insert(&CB);

indicatePessimisticFixpoint(); break;

return;

} }

// All other OpenMP runtime calls will not reach parallel regions so they // All other OpenMP runtime calls will not reach parallel regions so they

// can be safely ignored for now. Since it is a known OpenMP runtime call we // can be safely ignored for now. Since it is a known OpenMP runtime call we

// have now modeled all effects and there is no need for any update. // have now modeled all effects and there is no need for any update.

indicateOptimisticFixpoint(); indicateOptimisticFixpoint();

} }

ChangeStatus updateImpl(Attributor &A) override { ChangeStatus updateImpl(Attributor &A) override {

▲ Show 20 Lines • Show All 867 Lines • Show Last 20 Lines