-
Notifications
You must be signed in to change notification settings - Fork 15k
[WIP][DAG] visitFREEZE - always allow freezing multiple operands #152107
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
RKSimon
wants to merge
1
commit into
llvm:main
Choose a base branch
from
RKSimon:dag-freeze-multiple
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8365b9b to
f6e048a
Compare
8fc553e to
c80c4d2
Compare
715be61 to
0edc8b6
Compare
RKSimon
added a commit
to RKSimon/llvm-project
that referenced
this pull request
Aug 22, 2025
…te poison/undef Add AMDGPUTargetLowering::canCreateUndefOrPoisonForTargetNode handler and tag BFE_I32/U32 nodes as they can only propagate poison, not create poison/undef. Fighting some of the remaining regressions in llvm#152107 - need advice on the v_round_f64 change
0edc8b6 to
3b0856e
Compare
c7695aa to
4c5743a
Compare
RKSimon
added a commit
to RKSimon/llvm-project
that referenced
this pull request
Aug 26, 2025
…2P8AFFINEQB / GF2P8MULB handling All 3 instructions are well defined bit twiddling operations - they do not introduce undef/poison with well defined inputs. Fixes regressions in llvm#152107
4c5743a to
1d2fe46
Compare
b7dc70a to
c22f9a0
Compare
RKSimon
added a commit
to RKSimon/llvm-project
that referenced
this pull request
Aug 27, 2025
…NLOG handling Basic bitlogic operations don't create undef/poison. Its proving really annoying to create proper test coverage for these specific opcodes as they tend to appear in later stages - their presence does help in some upcoming patches (e.g. llvm#152107) and I'd prefer to get them committed early.
RKSimon
added a commit
that referenced
this pull request
Aug 27, 2025
…NLOG handling (#155600) Basic bitlogic operations don't create undef/poison. Its proving really annoying to create proper test coverage for these specific opcodes as they tend to appear in later stages - their presence does help in some upcoming patches (e.g. #152107) and I'd prefer to get them committed early.
c22f9a0 to
21092ff
Compare
3de4929 to
5c050e4
Compare
f3126ef to
900e674
Compare
0934039 to
d27ab57
Compare
Remove the limited freeze multiple operand handling, always freeze all operands and rely on later visitFREEZE calls to merge frozen/unfrozen versions of each node to prevent infinite loops. This also removes the special handling of frozen SRA/SRL nodes as most of the regressions are related Fixes llvm#149798 Fixes llvm#150204
d27ab57 to
0daa120
Compare
RKSimon
added a commit
to RKSimon/llvm-project
that referenced
this pull request
Sep 29, 2025
…directly freeze all operands Avoid the ReplaceAllUsesOfValueWith approach that has caused so many problems in previous attempts at this (llvm#145939). Minor step towards llvm#149798 - eventually we're just going to use this path for all node types, but there's a large number of regressions that need addressing first (see llvm#152107). This exposed a couple of things that need to be addressed here: - we need to revisit frozen nodes once we've merged all frozen/unfrozen uses of a node. - RISCVISD::READ_VLENB is never undef/poison Many of the current regressions are due to us not doing more to avoid freeze(load) nodes - if the load is legal, its no longer going to split. I'm not certain exactly when we can split nodes - llvm#160884 tried to catch up to ValueTracking but that might still be wrong.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove the limited freeze multiple operand handling, always freeze all operands and rely on later visitFREEZE calls to merge frozen/unfrozen versions of each node to prevent infinite loops.
This also removes the special handling of frozen SRA/SRL nodes as most of the regressions are related
Fixes #149798
Fixes #150204