-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
[ARM][ISel] Fix crash of ISD::FMINNUM/FMAXNUM #65849
Conversation
I think it should be the other way around. The instruction should be legal if HasFPARMv8 && HasNEON. It's a bit of a strange combination to have armv7+fp-armv8, but that appears to be what happens for the scalar instructions and from gcc. |
Thanks @davemgreen. Would you please show how can we infer that the instruction fmaxnum should be legal if HasFPARMv8 && HasNEON ?
|
Yeah can we change it to [HasFPARMv8, HasNEON]? |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the comments, this LGTM. Thanks for working on this.
The instruction of ISD::FMINNUM/FMAXNUM should be legal if HasFPARMv8 && HasNEON. For the combination of armv7+fp-armv8, armv7 imply the feature HasNEON on, and fp-armv8 matchs the feature HasFPARMv8, so it is legal. Fixes llvm#65820
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Some of the tests are showing some worse scalization but for what you are fixing this LGTM. Thanks for the fix.
The instruction of ISD::FMINNUM/FMAXNUM should be legal if HasFPARMv8 && HasNEON. For the combination of armv7+fp-armv8, armv7 imply the feature HasNEON on, and fp-armv8 matchs the feature HasFPARMv8, so it is legal Fixes llvm#65820
The instruction of ISD::FMINNUM/FMAXNUM should be legal if HasFPARMv8 && HasNEON. For the combination of armv7+fp-armv8, armv7 imply the feature HasNEON on, and fp-armv8 matchs the feature HasFPARMv8, so it is legal Fixes llvm#65820
The instruction of ISD::FMINNUM/FMAXNUM should be legal if HasFPARMv8 && HasNEON.
For the combination of armv7+fp-armv8, armv7 imply the feature HasNEON
on, and fp-armv8 matchs the feature HasFPARMv8, so it is legal
Fixes #65820