-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[IRGen] Correctly sext instead of zext for negative integer types #84655
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
Conversation
@swift-ci please smoke test |
lib/IRGen/MetadataRequest.cpp
Outdated
value = value.sextOrTrunc(IGM.SizeTy->getBitWidth()); | ||
} else { | ||
value = value.zextOrTrunc(IGM.SizeTy->getBitWidth()); | ||
} |
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.
This is exactly the same as just calling sextOrTrunc
.
IntegerType::getValue()
should probably document the value it returns, which appears to be that (1) it faithfully represents the written argument with arbitrary precision and (2) the bit width is the minimum necessary to represent the value correctly.
Also, since this has a trunc
component, where (if anywhere) do we actually diagnose an integer generic argument that's out of the expressible range of Int
?
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.
Yeah, but in a theoretical future where we support UInt
as values, then we'd need to come back here and use zextOrTrunc
no?
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.
Yeah, but in a theoretical future where we support UInt as values, then we'd need to come back here and use zextOrTrunc no?
No, not in this case.
You need to use sextOrTrunc
or zextOrTrunc
appropriately according to the signedness of the source value, not its sign. Signedness is not something that APInt
carries directly; isNegative()
is just returning whether the high bit is set and should generally not be used if you actually have a fixed-width unsigned value. But here, IntegerType::getValue()
is always returning an exact, arbitrary-width value with the correct sign bit for its mathematical value. Having a correct sign bit means it is a signed value, and it is always correct to use sextOrTrunc
on it.
@swift-ci please smoke test |
[IRGen] Correctly sext instead of zext for negative integer types
Resolves the issue defined here: https://forums.swift.org/t/integer-generics-negative-literals-seem-broken/82481