This is an archive of the discontinued LLVM Phabricator instance.

[LangRef] state that align assume op bundle may take an extra argument
AbandonedPublic

Authored by aqjune on Mar 16 2021, 1:11 AM.

Details

Reviewers
jdoerfert
Tyker
Summary

As discussed in D90529, this patch adds statements about "align"(i8* ptr, i64 a, i64 b).

Besides this, small non-functional edits are added:

  • Small edits to assume op bundle's lexical grammar
  • A clarification that align/nonnull also raises UB
  • etc

Diff Detail

Event Timeline

aqjune requested review of this revision.Mar 16 2021, 1:11 AM
aqjune created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2021, 1:11 AM

I like the additions. Some suggestions inlinined. TBH, the fact that we now have "multiple kinds of arguments" is confusing.
If you agree, maybe we should speak of "operand bundle tag and modifiers" or "operand bundle elements" throughout.

llvm/docs/LangRef.rst
2253
2270–2271
2281–2288
aqjune updated this revision to Diff 331142.Mar 16 2021, 6:31 PM

Address comments, minor update to the grammar

aqjune marked 3 inline comments as done.Mar 16 2021, 6:35 PM

the fact that we now have "multiple kinds of arguments" is confusing.

I think it seems okay - the first argument being the value to hold and other arguments being the values passed to the attribute seems clear and explicit.
Unless there exists a case that does not fit in this rule, it looks good.

gently ping

Tyker added a comment.Apr 1 2021, 7:21 AM

seems good to me. but maybe other still have comments.

aqjune abandoned this revision.Feb 22 2023, 8:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 22 2023, 8:18 AM