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 ` castIsValid(op, S, Ty) && "Invalid cast!"' failed. #57986

Closed
JonPsson opened this issue Sep 26, 2022 · 4 comments
Closed

Assertion ` castIsValid(op, S, Ty) && "Invalid cast!"' failed. #57986

JonPsson opened this issue Sep 26, 2022 · 4 comments
Assignees

Comments

@JonPsson
Copy link
Contributor

clang-16 -cc1 -triple s390x-ibm-linux -S -O2 -o /dev/null tc_crash0_aftercreduce.ll

...
#9 0x000002aa01a1de90 llvm::InstCombinerImpl::visitCallInst(llvm::CallInst&)
...

testcase.tar.gz

@rotateright
Copy link
Contributor

We're dying on this fold:
// umin(x, 1) == zext(x != 0)

Reduced IR test that fails with "opt -instcombine":

@g = external dso_local global [9 x i32], align 4

define i1 @PR57986() {
  %umin = call i1 @llvm.umin.i1(i1 ptrtoint (ptr @g to i1), i1 true)
  ret i1 %umin
}

@rotateright rotateright self-assigned this Sep 26, 2022
@rotateright
Copy link
Contributor

That fold in instcombine is awkward, but we can probably resolve this with a small change to -instsimplify.
Ie, we really should've killed that instruction before it tried to get transformed because "true" is the max value of i1.

rotateright added a commit that referenced this issue Sep 26, 2022
This shows the root problem that leads to the crash in issue #57986.
rotateright added a commit that referenced this issue Sep 26, 2022
…alls

The test shows that we would fail to consistently fold the
instruction based on the max value operand. This is also
the root cause for issue #57986, but I'll add an instcombine
test + assert for that exact problem in another commit.
rotateright added a commit that referenced this issue Sep 26, 2022
This is a test to verify that we do not crash with the
problem noted in issue #57986. The root problem should
be fixed with a prior change to InstSimplify.
@rotateright
Copy link
Contributor

@JonPsson - I think this is fixed with the above 3 commits (the 1st two are enough to avoid crashing). If you agree, we can either close or try to backport to the release branch. I have no preference on that (it is a crasher, but it's probably been there for a long time).

@JonPsson
Copy link
Contributor Author

Thanks for the quick fix! I have verified that the crash is gone also on the unreduced test case now (csmith).

I'll close this then. Don't know if it's worth back porting if it's been there a long time, but I guess it couldn't hurt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants