Skip to content

[DFAJumpThreading] crash since b167ada #106083

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.

Sign up for GitHub

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

Jump to bottom
Closed
JonPsson1 opened this issue Aug 26, 2024 · 2 comments · Fixed by #109511
Closed

[DFAJumpThreading] crash since b167ada #106083

JonPsson1 opened this issue Aug 26, 2024 · 2 comments · Fixed by #109511
Assignees
@UsmanNadeem
Labels
crash Prefer [crash-on-valid] or [crash-on-invalid] llvm:transforms

Comments

@JonPsson1
Copy link
Contributor

Seeing many assertions that go away if I revert b167ada "[DFAJumpThreading] Rewrite the way paths are enumerated (#96127)".

clang-20 -cc1 -triple s390x-unknown-linux-gnu -target-cpu z15 -O3 -w -mllvm -enable-dfa-jump-thread -o /dev/null -emit-llvm -x ir  tc_dfa-jt_invalidbbarg.ll
...
10 0x000002aa0a73e544 (anonymous namespace)::DFAJumpThreading::run
...

clang-20 -cc1 -triple s390x-unknown-linux-gnu   -target-cpu z16 -O3 -w -mllvm -enable-dfa-jump-thread -o /dev/null -emit-llvm -x ir  tc_select_mustbedeadnow.ll
...
#10 0x000002aa1b0c0968 llvm::DFAJumpThreadingPass::run
...

tcs_dfa-jt.tar.gz
@EugeneZelenko EugeneZelenko added llvm:optimizations crash Prefer [crash-on-valid] or [crash-on-invalid] and removed new issue labels Aug 26, 2024
@mikaelholmen
Copy link
Collaborator

Perhaps the same assertion I reported here:
#96127 (comment)
?

@JonPsson1
Copy link
Contributor Author

Perhaps the same assertion I reported here: #96127 (comment) ?

Yes, same assertion, and in addition my second test case above triggered another assertion.

@UsmanNadeem UsmanNadeem mentioned this issue Aug 28, 2024
Open
@mikaelholmen mikaelholmen mentioned this issue Sep 2, 2024
Merged
UsmanNadeem added a commit to UsmanNadeem/llvm-project that referenced this issue Sep 21, 2024
@UsmanNadeem
[DFAJumpThreading] Handle select unfolding when user phi is not a dir…
91de2ec 
…ect successor

Previously the code assumed that the select instruction is defined
in a block that is a direct predecessor of the block where the PHINode
uses it. So, we were hitting an assertion when we tried to access the
def block as an incoming block for the user phi node.

This patch handles that case by using the correct end block
and creating a new phi node that aggregates both the values of the
select in that end block, and then using that new unfolded phi to
overwrite the original user phi node.

Fixes llvm#106083

Change-Id: Ie471994cca232318f74a6e6438efa21e561c2dc0
@UsmanNadeem UsmanNadeem mentioned this issue Sep 21, 2024
Merged
UsmanNadeem added a commit to UsmanNadeem/llvm-project that referenced this issue Sep 23, 2024
@UsmanNadeem
[DFAJumpThreading] Handle select unfolding when user phi is not a dir…
9f0d042 
…ect successor

Previously the code assumed that the select instruction is defined
in a block that is a direct predecessor of the block where the PHINode
uses it. So, we were hitting an assertion when we tried to access the
def block as an incoming block for the user phi node.

This patch handles that case by using the correct end block
and creating a new phi node that aggregates both the values of the
select in that end block, and then using that new unfolded phi to
overwrite the original user phi node.

Fixes llvm#106083

Change-Id: Ie471994cca232318f74a6e6438efa21e561c2dc0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@UsmanNadeem UsmanNadeem

Labels
crash Prefer [crash-on-valid] or [crash-on-invalid] llvm:transforms
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[DFAJumpThreading] Handle select unfolding when user phi is not a dir… UsmanNadeem/llvm-project
5 participants
@EugeneZelenko @UsmanNadeem @mikaelholmen @JonPsson1 and others