Skip to content

[ValueTracking] Filter out non-interesting conditions #118493

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Dec 3, 2024

Address issue #117442 (comment)

class DomConditionCache {
private:
/// A map of values about which a branch might be providing information.
using AffectedValuesMap = DenseMap<Value *, SmallVector<BranchInst *, 1>>;
using AffectedValuesMap =
Copy link
Contributor

@andjo403 andjo403 Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as it seems like conditionsFor is called for one DomConditionFlag value at a time maybe better add the flag as en argument and change map to store the flag as part of the key to the map?
DenseMap<std::pair<Value *, DomConditionFlag>, SmallVector<BranchInst *, 1>>;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm noticed now that you mentioned that it was the calls to registerBranch that was the problem in #117442 (comment) and this will result in more work then.
but if it is the registerBranch that is the problem I assume that any filtering added will result in worse compile time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From some quick profiling, the impact isn't from registerBranch, but from extra DT queries in computeKnownBits. I think in principle this patch should still help as long as we don't add comparisons where both sides are variables to the KnownBits category (338d7fc#r152360307).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dtcxzyw dtcxzyw force-pushed the perf/domcond-flags branch from 652a01a to 4b0feb4 Compare December 7, 2024 07:10
dtcxzyw added a commit that referenced this pull request Apr 18, 2025
This patch avoids adding RHS for comparisons with two variable operands
(#118493 (comment)).
Instead, we iterate over related dominating conditions of both V1 and V2
in `isKnownNonEqualFromContext`, as suggested by goldsteinn
(#117442 (comment)).

Compile-time improvement:
https://llvm-compile-time-tracker.com/compare.php?from=c6d95c441a29a45782ff72d6cb82839b86fd0e4a&to=88464baedd7b1731281eaa0ce4438122b4d218a7&stat=instructions:u
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Apr 18, 2025
…7388)

This patch avoids adding RHS for comparisons with two variable operands
(llvm/llvm-project#118493 (comment)).
Instead, we iterate over related dominating conditions of both V1 and V2
in `isKnownNonEqualFromContext`, as suggested by goldsteinn
(llvm/llvm-project#117442 (comment)).

Compile-time improvement:
https://llvm-compile-time-tracker.com/compare.php?from=c6d95c441a29a45782ff72d6cb82839b86fd0e4a&to=88464baedd7b1731281eaa0ce4438122b4d218a7&stat=instructions:u
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
This patch avoids adding RHS for comparisons with two variable operands
(llvm#118493 (comment)).
Instead, we iterate over related dominating conditions of both V1 and V2
in `isKnownNonEqualFromContext`, as suggested by goldsteinn
(llvm#117442 (comment)).

Compile-time improvement:
https://llvm-compile-time-tracker.com/compare.php?from=c6d95c441a29a45782ff72d6cb82839b86fd0e4a&to=88464baedd7b1731281eaa0ce4438122b4d218a7&stat=instructions:u
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
This patch avoids adding RHS for comparisons with two variable operands
(llvm#118493 (comment)).
Instead, we iterate over related dominating conditions of both V1 and V2
in `isKnownNonEqualFromContext`, as suggested by goldsteinn
(llvm#117442 (comment)).

Compile-time improvement:
https://llvm-compile-time-tracker.com/compare.php?from=c6d95c441a29a45782ff72d6cb82839b86fd0e4a&to=88464baedd7b1731281eaa0ce4438122b4d218a7&stat=instructions:u
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
This patch avoids adding RHS for comparisons with two variable operands
(llvm#118493 (comment)).
Instead, we iterate over related dominating conditions of both V1 and V2
in `isKnownNonEqualFromContext`, as suggested by goldsteinn
(llvm#117442 (comment)).

Compile-time improvement:
https://llvm-compile-time-tracker.com/compare.php?from=c6d95c441a29a45782ff72d6cb82839b86fd0e4a&to=88464baedd7b1731281eaa0ce4438122b4d218a7&stat=instructions:u
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants