[PATCH] D90275: [clang][IR] Add support for leaf attribute

jdoerfert added a comment. In D90275#3715563 , @xbolva00 wrote: > Was there any follow up? It seems that this hint (attribute) is currently not > used by LLVM passes. It is, also in our runtime, see https://reviews.llvm.org/D131628#3716252 Repository:

[PATCH] D90275: [clang][IR] Add support for leaf attribute

xbolva00 added a comment. Was there any follow up? It seems that this hint (attribute) is currently not used by LLVM passes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. Added the description in https://reviews.llvm.org/D131628 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 ___ cfe-commits mailing list

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. In D90275#3712991 , @nlopes wrote: > In D90275#3712969 , @gulfem wrote: > >> In D90275#3712880 , @nlopes wrote: >> >>> In D90275#3707793

[PATCH] D90275: [clang][IR] Add support for leaf attribute

nlopes added a comment. In D90275#3712969 , @gulfem wrote: > In D90275#3712880 , @nlopes wrote: > >> In D90275#3707793 , @nlopes wrote: >> >>> In D90275#3671983

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. In D90275#3712880 , @nlopes wrote: > In D90275#3707793 , @nlopes wrote: > >> In D90275#3671983 , @gulfem wrote: >> >>> In D90275#3671019

[PATCH] D90275: [clang][IR] Add support for leaf attribute

nlopes added a comment. In D90275#3707793 , @nlopes wrote: > In D90275#3671983 , @gulfem wrote: > >> In D90275#3671019 , @nlopes wrote: >> >>> Could you please add a

[PATCH] D90275: [clang][IR] Add support for leaf attribute

nlopes added a comment. In D90275#3671983 , @gulfem wrote: > In D90275#3671019 , @nlopes wrote: > >> Could you please add a description of this attribute to LangRef? >> We need the semantics of this. >> >> Thank

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. In D90275#3671019 , @nlopes wrote: > Could you please add a description of this attribute to LangRef? > We need the semantics of this. > > Thank you! Sure, I'll do that! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D90275: [clang][IR] Add support for leaf attribute

nlopes added a comment. Herald added a subscriber: ormris. Herald added a project: All. Could you please add a description of this attribute to LangRef? We need the semantics of this. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem closed this revision. gulfem added a comment. Since this patch has been merged and the issue in the test has been resolved, I'm closing this review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. > I fixed it in > https://reviews.llvm.org/D92493 > But that was just to make my test pass. Ok, thank you! I did not realize that typo after the merge, and sorry about that! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

Re: [PATCH] D90275: [clang][IR] Add support for leaf attribute

yes. Also #43 should be #42 I fixed it in https://reviews.llvm.org/D92493 But that was just to make my test pass. On Wed, Dec 16, 2020 at 11:35 AM Gulfem Savrun Yeniceri via Phabricator < revi...@reviews.llvm.org> wrote: > gulfem added a comment. > > > The problem in

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. > The problem in `llvm/test/Bitcode/attributes.ll` is clear? Instead of `define void @f70() nocallback`, it should be `define void @f69() nocallback`, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/

[PATCH] D90275: [clang][IR] Add support for leaf attribute

xur added a comment. This breaks llvm/test/Bitcode/attributes.ll. The added test code was obviously wrong. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 ___

[PATCH] D90275: [clang][IR] Add support for leaf attribute

jdoerfert added a comment. In D90275#2453641 , @gulfem wrote: >> This is missing a lang ref entry for `nocallback` and the `attributes.ll` >> test is arguably broken (see below). > > Could you please elaborate on missing lang ref entry? Where that should

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. > This is missing a lang ref entry for `nocallback` and the `attributes.ll` > test is arguably broken (see below). Could you please elaborate on missing lang ref entry? Where that should be added? > The "definition" in `llvm/include/llvm/IR/Attributes.td` (see below),

[PATCH] D90275: [clang][IR] Add support for leaf attribute

jdoerfert reopened this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Apologies for being silent for too long. This is missing a lang ref entry for `nocallback` and the `attributes.ll` test is arguably broken (see below). The "definition" in

[PATCH] D90275: [clang][IR] Add support for leaf attribute

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7c0e3a77bc43: [clang][IR] Add support for leaf attribute (authored by gulfem, committed by haowei). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 310290. gulfem marked an inline comment as done. gulfem added a comment. Add a new line at the end of the test file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added a comment. Thanks for the new tests! Comment at: clang/test/Sema/attr-leaf.c:13 +} \ No newline at end of file Can you add the newline to the end of the file? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as done. gulfem added inline comments. Comment at: clang/test/Sema/attr-leaf.c:4 + +void f() __attribute__((leaf)); aaron.ballman wrote: > Can you also add tests that show the attribute diagnoses being written on the > wrong

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 310269. gulfem added a comment. Added more Sema test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/include/clang/Basic/Attr.td

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a testing request, the frontend parts LGTM. I don't know enough about the LLVM side to feel comfortable signing off on it. @jdoerfert, would you mind approving that

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: > gulfem wrote: > > aaron.ballman wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 310052. gulfem added a comment. Only support leaf attribute in functions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/include/clang/Basic/Attr.td

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as not done. gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote:

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > gulfem wrote: > > gulfem wrote: > > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as not done. gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: > gulfem wrote: > > aaron.ballman wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: > gulfem wrote: > > aaron.ballman wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as done. gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as done. gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 303200. gulfem added a comment. Remove direct from description and ObjCMethod for Obj functionality Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files:

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aqjune added a comment. In D90275#2374648 , @gulfem wrote: > Leaf attribute is specifically intended for library functions and I think all > the existing usage of leaf attribute is in the library function declarations. > For ex, it is only used in

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. In D90275#2371343 , @jdoerfert wrote: > The more I think about it, the more I think we should never create a > `leaf`/`nocallback` definition. Only declarations should carry that attribute. Leaf attribute is specifically intended

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: > gulfem wrote: > > aaron.ballman wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aqjune added a comment. In D90275#2371813 , @jdoerfert wrote: > As noted by the GCC docs, it doesn't mean anything on a definition so that > you can safely merge TUs. I want us to forbid `leaf` on IR function > definitions for that reason, it would not

[PATCH] D90275: [clang][IR] Add support for leaf attribute

mcgrathr added a comment. The GCC documentation specifically gives the example of the standard C function `qsort` as one that does not qualify as `__attribute__((leaf))` because it uses a callback function (that presumably might be from the caller's own TU). AIUI the claim the attribute makes

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3910 +in library functions. Functions marked with the ``leaf`` attribute are not allowed +to jump back into the caller's translation unit, whether through invoking a +callback function, a direct

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 302716. gulfem added a comment. Add a target into the test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/include/clang/Basic/Attr.td

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as done. gulfem added a comment. In D90275#2371343 , @jdoerfert wrote: > The more I think about it, the more I think we should never create a > `leaf`/`nocallback` definition. Only declarations should carry that attribute.

[PATCH] D90275: [clang][IR] Add support for leaf attribute

jdoerfert added a comment. In D90275#2371764 , @aqjune wrote: > Hi, > > Naming is a hard thing... I have no special preference. :/ > > However, I'd like to understand the details of this attribute. > > Would LTO be affected because `leaf` is guaranteed to

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aqjune added a comment. Hi, Naming is a hard thing... I have no special preference. :/ However, I'd like to understand the details of this attribute. Would LTO be affected because `leaf` is guaranteed to untouch the current translation unit only? // a.c int x; void f1() { f2(); }

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added a comment. In D90275#2371343 , @jdoerfert wrote: > The more I think about it, the more I think we should never create a > `leaf`/`nocallback` definition. Only declarations should carry that attribute. When talking about the IR that

[PATCH] D90275: [clang][IR] Add support for leaf attribute

jdoerfert added subscribers: aqjune, fhahn, efriedma, reames. jdoerfert added a comment. The more I think about it, the more I think we should never create a `leaf`/`nocallback` definition. Only declarations should carry that attribute. I'm also still not convinced `nocallback` is a good name.

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3910 +in library functions. Functions marked with the ``leaf`` attribute are not allowed +to jump back into the caller's translation unit, whether through invoking a +callback function, a

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as done. gulfem added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3909 +The ``leaf`` attribute is used as a compiler hint to improve dataflow analysis in library functions. +Functions marked as ``leaf`` attribute are not

[PATCH] D90275: [clang][IR] Add support for leaf attribute

jdoerfert added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3910 +Functions marked as ``leaf`` attribute are not allowed to enter their caller's translation unit. +Therefore, they cannot use or modify any data that does not escape the current compilation

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3910 +in library functions. Functions marked with the ``leaf`` attribute are not allowed +to jump back into the caller's translation unit, whether through invoking a +callback function, a direct

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 302369. gulfem added a comment. Update the attribute documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/include/clang/Basic/Attr.td

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > Should this

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; aaron.ballman wrote: > Should this attribute also be supported on

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3909 +The ``leaf`` attribute is used as a compiler hint to improve dataflow analysis in library functions. +Functions marked as ``leaf`` attribute are not allowed to enter their caller's

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem marked an inline comment as not done. gulfem added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3909 +The ``leaf`` attribute is used as a compiler hint to improve dataflow analysis in library functions. +Functions marked as ``leaf`` attribute are

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 302349. gulfem marked 3 inline comments as done. gulfem added a comment. Addressed the comments about the documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; Should this attribute also be supported on things like

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 301789. gulfem added a comment. Fix the typo in the test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/include/clang/Basic/Attr.td

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem updated this revision to Diff 301787. gulfem added a comment. Add IR and bitcode tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 Files: clang/include/clang/Basic/Attr.td

[PATCH] D90275: [clang][IR] Add support for leaf attribute

leonardchan added a comment. I'd recommend splitting this into two patches, one for LLVM changes and another for Clang changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275

[PATCH] D90275: [clang][IR] Add support for leaf attribute

jdoerfert added a comment. In D90275#2357951 , @gulfem wrote: > In D90275#2357945 , @jdoerfert wrote: > >> In D90275#2357931 , @mcgrathr wrote: >> >>> There is an RFC going

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem added a comment. In D90275#2357945 , @jdoerfert wrote: > In D90275#2357931 , @mcgrathr wrote: > >> There is an RFC going out with this prototype as reference. When there is >> consensus on the RFC, this

[PATCH] D90275: [clang][IR] Add support for leaf attribute

jdoerfert added a comment. In D90275#2357931 , @mcgrathr wrote: > There is an RFC going out with this prototype as reference. When there is > consensus on the RFC, this will get in shape for landing with complete tests > and all. Great. I didn't find

[PATCH] D90275: [clang][IR] Add support for leaf attribute

mcgrathr added a comment. There is an RFC going out with this prototype as reference. When there is consensus on the RFC, this will get in shape for landing with complete tests and all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/

[PATCH] D90275: [clang][IR] Add support for leaf attribute

jdoerfert added a comment. (To clarify, I am not opposing this, my comment before was brief but not meant to be negative. I'm generally for new attributes and this one is already in existing source so that is even better.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D90275: [clang][IR] Add support for leaf attribute

jdoerfert added a comment. Tests missing (on all levels), was there an RFC? What is the expected usage of this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275

[PATCH] D90275: [clang][IR] Add support for leaf attribute

gulfem created this revision. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, jdoerfert, steven_wu, hiraditya. Herald added a reviewer: aaron.ballman. Herald added projects: clang, LLVM. gulfem requested review of this revision. GCC supports __attribute__((leaf)) as an