Page MenuHomePhabricator

[builtins] Add __builtin_assume_separate_storage.
Needs RevisionPublic

Authored by davidtgoldblatt on Oct 21 2022, 4:37 PM.

Details

Reviewers
hfinkel
bruno
Summary

Just plumbing from the language level to the assume intrinsics with
separate_storage operand bundles.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptOct 21 2022, 4:37 PM
Matt added a subscriber: Matt.Oct 25 2022, 11:20 AM

Updating per review comments.

davidtgoldblatt retitled this revision from [builtins] add separate storage builtin support. to [builtins] Add __builtin_assume_separate_storage..
davidtgoldblatt edited the summary of this revision. (Show Details)

Updating per review comments.

davidtgoldblatt published this revision for review.Nov 17 2022, 4:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 17 2022, 4:53 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Fix out-of-date comment.

Per review comments.

bruno requested changes to this revision.Dec 20 2022, 6:34 AM

Nice, thanks for adding the builtin layer.

clang/lib/Sema/SemaChecking.cpp
7808

Can you prefix this with a FIXME/TODO?

7810

IIUC, seems like there a two pieces here you can already cover as part of this patch:

  • Check for exact two arguments. If in the future an optional multi-arg variadic version is to be supported the checking could be enhanced/changed to support that.
  • Check both are pointers. I noticed that in the testcase you rely on incompatible integer to pointer conversion and friends to catch some of these, but a more user friendly diagnostic could be explicit about the expectations. In case these current error diagnostics you are relying run before this sema check, perhaps a note diagnostic could help clarify.
This revision now requires changes to proceed.Dec 20 2022, 6:34 AM
bruno added a comment.Dec 20 2022, 6:44 AM

Additionally, this likely deserves an entry in ./clang/docs/ReleaseNotes.rst.