[Sema][CodeGen] Support __builtin_<op>_overflow with __intcap #747
+5,681
−30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrtc27
force-pushed
the
builtin-op-overflow
branch
from
1d08433
to
89bb014
Compare
arichardson
reviewed
Jul 31, 2024
clang/lib/CodeGen/CGBuiltin.cpp
Outdated
| struct WidthAndSignedness { | ||
| unsigned Width; | ||
| struct RangeAndSignedness { | ||
| unsigned Range; |
There was a problem hiding this comment.
I think it would be good to upstream this NFC rename to reduce the diff.
There was a problem hiding this comment.
Hm, I was confused and our getIntWidth in fact gives the range, which I guess makes sense in C land where this is. Our getIntRange is just a wrapper for getIntWidth, and I suppose is a stalled effort to distinguish the two, which likely needs a hell of a lot more work and commitment from upstream to adopt. So probably this renaming should just be dropped?
There was a problem hiding this comment.
Still worth upstreaming llvm/llvm-project#101765 as a minor thing though
arichardson
reviewed
Aug 1, 2024
This will be reused for __builtin_<op>_overflow, which gets checked in SemaChecking.cpp instead and so can't currently use this.
jrtc27
force-pushed
the
builtin-op-overflow
branch
from
89bb014
to
def11ee
Compare
|
Width -> Rename dropped, existing Sema code tweaked and exposed, and ProvenanceCap now using ConstantPointerNull |
Morello LLVM has downstream support for this, but it's both incomplete (see https://git.morello-project.org/morello/llvm-project/-/issues/80) and incorrect with regards to provenance (in that it takes a naive type-based approach rather than considering the cheri_no_provenance attribute, meaning it differs from the binary operators in provenance semantics). This is a from-scratch implementation that aims to not have the same shortcomings.
jrtc27
force-pushed
the
builtin-op-overflow
branch
from
def11ee
to
937cbb9
Compare
arichardson
approved these changes
Aug 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Morello LLVM has downstream support for this, but it's both incomplete
(see https://git.morello-project.org/morello/llvm-project/-/issues/80)
and incorrect with regards to provenance (in that it takes a naive
type-based approach rather than considering the cheri_no_provenance
attribute, meaning it differs from the binary operators in provenance
semantics). This is a from-scratch implementation that aims to not have
the same shortcomings.