Skip to content
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

Assertion `getMinSignedBits() <= 64 && "Too many bits for int64_t"' failed in Loop Strength Reduction with new SCEV-based salvaging #50671

Closed
JosephTremoulet opened this issue Aug 3, 2021 · 10 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla

Comments

@JosephTremoulet
Copy link
Member

Bugzilla Link 51329
Resolution FIXED
Resolved on Oct 11, 2021 20:29
Version trunk
OS Linux
Blocks #51489
Attachments Small function that triggers the assertion failure in LSR
CC @tstellar
Fixed by commit(s) 21ee38e 8988ce3

Extended Description

This assertion has been failing in some of our downstream tests each time D105207 gets checked in. I've verified that it fails at 0ba8595 but not at its parent. It also currently repros on godbolt with "assertions trunk": https://godbolt.org/z/n6T3zrsvY

Reproducer testcase attached.

@JosephTremoulet
Copy link
Member Author

assigned to @chrisjbris

@JosephTremoulet
Copy link
Member Author

I've verified that this still fails after 2537120 as well.

@JosephTremoulet
Copy link
Member Author

I forgot to mention the command line I'm using to reproduce the issue with the attached file. It's just llc repro.ll.

@chrisjbris
Copy link

Thanks for the comprehensive report. It looks pretty obvious what the culprit is. I will add a test to make sure APInt->getSextValue() isn't called for integers greater than INTMAX and hopefully that will do the trick for now.

@chrisjbris
Copy link

Fixed with 21ee38e

@JosephTremoulet
Copy link
Member Author

Fix verified, thanks.

@tstellar
Copy link
Collaborator

tstellar commented Aug 5, 2021

This does not cherry-pick cleanly, can someone backport it and either attach a patch or push a branch to a personal git repo on github.

@chrisjbris
Copy link

I attempted to cherry-pick the series to the release branch myself and each was clean. Three commits were necessary, which I described here: https://reviews.llvm.org/D105207. I think its not clear from this bug alone which commits are required. pehaps that was the reason for the un-clean cherry-pick attempt.

The three cherry-picks together built & passed the lit suite cleanly so I have pushed to the release branch, with original commit hashes added to the messages.

@tstellar
Copy link
Collaborator

tstellar commented Aug 5, 2021

Merged: 8988ce3

@tstellar
Copy link
Collaborator

mentioned in issue #51489

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

3 participants