This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Implement P1007R3: std::assume_aligned
ClosedPublic

Authored by ldionne on Feb 3 2022, 12:24 PM.

Details

Reviewers
philnik
Group Reviewers
Restricted Project
Commits
rGc292b6066cca: [libc++] Implement P1007R3: std::assume_aligned
Summary

This supersedes and incoroporates content from both D108906 and D54966,
and also some original content.

Co-Authored-by: Marshall Clow <mclow.lists@gmail.com>
Co-Authored-by: Gonzalo Brito Gadeschi

Diff Detail

Event Timeline

ldionne created this revision.Feb 3 2022, 12:24 PM
ldionne requested review of this revision.Feb 3 2022, 12:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 3 2022, 12:24 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne updated this revision to Diff 405747.Feb 3 2022, 12:30 PM

Add the feature-test macro.

Have the issues described in D54966 been resolved? If yes, this LGTM.

libcxx/test/std/utilities/memory/ptr.align/assume_aligned.nodiscard.verify.cpp
21

Why is this [[maybe_unused]]?

ldionne marked an inline comment as done.Apr 8 2022, 11:45 AM

Have the issues described in D54966 been resolved? If yes, this LGTM.

I believe so, because we don't use __builtin_assume_aligned during constant evaluation anymore, and that was the main holdup IIUC.

libcxx/test/std/utilities/memory/ptr.align/assume_aligned.nodiscard.verify.cpp
21

Not sure, removed.

Herald added a project: Restricted Project. · View Herald TranscriptApr 8 2022, 11:45 AM
ldionne updated this revision to Diff 421605.Apr 8 2022, 11:45 AM
ldionne marked an inline comment as done.

Rebase onto main and address a few comments. I'll ship this once CI passes.

ldionne accepted this revision as: Restricted Project.Apr 8 2022, 11:46 AM
ldionne updated this revision to Diff 421620.Apr 8 2022, 12:54 PM

Address CI issue.

philnik accepted this revision.Apr 8 2022, 1:40 PM

LGTM % nit

libcxx/include/__memory/assume_aligned.h
38

I think I'd like a reinterpret_cast here a bit more. At least say that you're an evil person.

This revision is now accepted and ready to land.Apr 8 2022, 1:40 PM
ldionne marked an inline comment as done.Apr 11 2022, 7:46 AM

Applied @philnik's comment locally. The test failure is a fluke, so I'll ignore it. Shipping this!

This revision was landed with ongoing or failed builds.Apr 11 2022, 7:47 AM
This revision was automatically updated to reflect the committed changes.