[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

jdoerfert added a subscriber: probinson. jdoerfert added a comment. Found the commit: dcbe35bad518 The way I see it we just have to teach the inliner about `optnone` so we can uncouple the two (`optnone` and `noinline`). @probinson WDTY? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

jdoerfert added a comment. In D70366#1972880 , @LevitatingLion wrote: > In D70366#1971137 , @dexonsmith > wrote: > > > In D70366#1970758 , @jdoerfert > > wrote: > > > > >

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

LevitatingLion added a comment. In D70366#1971137 , @dexonsmith wrote: > In D70366#1970758 , @jdoerfert wrote: > > > In D70366#1970526 , @dexonsmith > > wrote: > > > > > In

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

dexonsmith added a comment. In D70366#1970758 , @jdoerfert wrote: > In D70366#1970526 , @dexonsmith > wrote: > > > In D70366#1970299 , > > @LevitatingLion wrote: > > > > >

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

jdoerfert added a comment. In D70366#1970526 , @dexonsmith wrote: > In D70366#1970299 , @LevitatingLion > wrote: > > > Maybe we can add an additional string attribute when adding the noinline > > attribute to func

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

dexonsmith added a comment. In D70366#1970526 , @dexonsmith wrote: > In D70366#1970299 , @LevitatingLion > wrote: > > > Maybe we can add an additional string attribute when adding the noinline > > attribute to fun

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

dexonsmith added a comment. In D70366#1970299 , @LevitatingLion wrote: > Maybe we can add an additional string attribute when adding the noinline > attribute to functions which are not marked noinline in the source code, > something like "noinline-added

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

dexonsmith added a comment. In D70366#1970375 , @jdoerfert wrote: > TBH, I would issue a warning if we see `flatten` in O0 that says this will > not work and be done with it. I would argue against diagnostics that depend on optimization level, since tha

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

jdoerfert added a comment. In D70366#1970299 , @LevitatingLion wrote: > While adding tests to clang I realized the attribute is not working as > intended when using an optimization level of zero, because clang adds the > noinline attribute to all functi

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

LevitatingLion added a comment. While adding tests to clang I realized the attribute is not working as intended when using an optimization level of zero, because clang adds the noinline attribute to all functions. In this case the optimizer cannot distinguish between functions originally marked

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

erichkeane added a comment. The semantics of this llvm attribute seem to better match 'flatten'. However, it is unfortunate that this doesn't change any clang tests. Can you add/alter a test in clang to validate the IR? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70366/new/ https:

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

lebedev.ri added a comment. In D70366#1960300 , @jdoerfert wrote: > I'm fine with this. I would hope a C/C++/Clang person will also take a look > though. This is missing clang codegen test[s]. Seems to look fine to me otherwise. CHANGES SINCE LAST ACT

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

jdoerfert added a comment. I'm fine with this. I would hope a C/C++/Clang person will also take a look though. Comment at: llvm/docs/LangRef.rst:1398 +This attribute is similar to ``alwaysinline``, but also applies recursively to +all inlined function calls. ``builti

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

LevitatingLion updated this revision to Diff 254217. LevitatingLion added a comment. Herald added a reviewer: sstefan1. I rebased my changes onto 49d00824bbb , renamed the attribute to 'alwaysinline_recursively', and added some

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

LevitatingLion added a comment. Thanks for the ping. I hadn't looked at this since, but I'll update the patch this weekend. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70366/new/ https://reviews.llvm.org/D70366 ___

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

jdoerfert added a comment. Herald added a subscriber: kerbowa. [reverse ping] Is this still active? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70366/new/ https://reviews.llvm.org/D70366 ___ cfe-comm

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

jdoerfert added a comment. We need more tests here. For one, the flatten attribute is not necessary to pass the test. Second, we need to check the corner cases, e.g. reduction with different cycle lengths. Comment at: llvm/docs/LangRef.rst:1428 can prove that the call/in

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

LevitatingLion added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70366/new/ https://reviews.llvm.org/D70366 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

LevitatingLion added inline comments. Comment at: llvm/docs/LangRef.rst:1428 can prove that the call/invoke cannot call a convergent function. +``flatten`` +This attribute is similar to ``alwaysinline``, but applies recursively to arsenm wrote: > It's no

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

lebedev.ri added a comment. Thank you for working on this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70366/new/ https://reviews.llvm.org/D70366 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

arsenm added inline comments. Comment at: llvm/docs/LangRef.rst:1428 can prove that the call/invoke cannot call a convergent function. +``flatten`` +This attribute is similar to ``alwaysinline``, but applies recursively to It's not obvious to me what the

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

LevitatingLion created this revision. LevitatingLion added reviewers: jdoerfert, pcc, chandlerc. LevitatingLion added projects: LLVM, clang. Herald added subscribers: dexonsmith, steven_wu, haicheng, hiraditya, eraman, nhaehnle, jvesely, mehdi_amini, arsenm. This adds a new 'flatten' attribute, w