Skip to content

Conversation

@dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Dec 3, 2024

Address issue #117442 (comment)

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
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