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

Thu Nov 5 11:38:53 PST 2020

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:
> gulfem wrote:
> > aaron.ballman wrote:
> > > gulfem wrote:
> > > > aaron.ballman wrote:
> > > > > Should this attribute also be supported on things like ObjC method decls or other function-like interfaces?
> > > > Do I need to do anything else to support this attribute in Objective-C as well?
> > > > I think we should support it in all the C languages family.
> > > >I think we should support it in all the C languages family.
> > > 
> > > That's already happening automatically -- there's a C and C++ spelling available for it and the attribute doesn't specify that it requires a particular language mode or target.
> > > 
> > > > Do I need to do anything else to support this attribute in Objective-C as well?
> > > You can add multiple subjects to the list here, so you can have this apply to `Function, ObjCMethod` for both of those. Another one to consider is whether this attribute can be written on a block declaration (like a lambda, but with different syntax). Beyond that, it's mostly just documentation, devising the test cases to ensure the ObjC functionality behaves as expected, possibly some codegen changes, etc.
> > AFAIK, users can specify function attributes in lambda expressions.
> > Lambda functions can only be accessed/called by the functions in the same translation unit, right?
> > Leaf attribute does not have any effect on the functions that are defined in the same translation unit.
> > For this reason, I'm thinking that leaf attribute would not have any effect if they are used in lambda expressions.
> > Do you agree with me?
> > AFAIK, users can specify function attributes in lambda expressions.
> 
> I always forget that you can do that for declaration attributes using GNU-style syntax...
> 
> > Lambda functions can only be accessed/called by the functions in the same translation unit, right?
> 
> Not necessarily, you could pass one across TU boundaries like a function pointer, for instance. e.g.,
> ```
> // TU1.cpp
> void foo() {
>   auto l = []() { ... };
>   bar(l);
> }
> 
> // TU2.cpp
> void bar(auto func) {
>   func();
> }
> ```
> Not necessarily, you could pass one across TU boundaries like a function pointer, for instance. e.g.,
As I mentioned before, 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 this reason, I think we do not need to support them for lambdas. Is that reasonable?

================
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 external function call, use of ``longjmp``, or other means.
----------------
aaron.ballman wrote:
> gulfem wrote:
> > aaron.ballman wrote:
> > > gulfem wrote:
> > > > I think this property is transitive. 
> > > > If a leaf function somehow enters into caller's translation unit (either via direct call or a via its call chain), it will violate the rule that says "Calls to external functions with this attribute must return to the current compilation unit only by return or by exception handling". 
> > > > Entering via its call chain is not a return or an exception handling.
> > > > Do you agree with that?
> > > > I think this property is transitive. ... Do you agree with that?
> > > 
> > > That makes sense to me! I think I'd recommend a slight modification to the docs then:
> > > 
> > > ```
> > > 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, possibly transitive, external function call, use of ``longjmp``, or other means.
> > > ```
> > > The last question is: by "direct" does the GCC docs imply that calls back to the caller's TU through a function *pointer* are not UB? I'd imagine those would also be bad, but I'd like to make sure (and perhaps we can remove the `direct` from the wording if calls through function pointers are disallowed).
> > I think the idea is that the control-flow should never come back to the caller's translation unit by any kind of control-flow changing mechanism (direct/indirect and call/jump).
> > The compiler might use this hint to do optimizations based on that assumption, so the user should ensure that.
> > Could you please explain what do you mean by calling back via function pointer?
> > I thought that callback function is basically calling back via a function pointer. 
> > Could you please explain what do you mean by calling back via function pointer?
> > I thought that callback function is basically calling back via a function pointer.
> 
> I had confused myself -- I think @jdoerfert answered my confusion in an earlier comment and I just wasn't thinking about that case hard enough. I think we want to drop the word "direct" from "external function call" because it doesn't matter whether the user says:
> ```
> void bar(void); // Assume calling this is bad
> bar();
> ```
> and
> ```
> void bar(void); // Assume calling this is bad
> void (*fp)(void) = bar;
> fp();
> ```
> > Could you please explain what do you mean by calling back via function pointer?
> > I thought that callback function is basically calling back via a function pointer.
> 
> I had confused myself -- I think @jdoerfert answered my confusion in an earlier comment and I just wasn't thinking about that case hard enough. I think we want to drop the word "direct" from "external function call" because it doesn't matter whether the user says:
> ```
> void bar(void); // Assume calling this is bad
> bar();
> ```
> and
> ```
> void bar(void); // Assume calling this is bad
> void (*fp)(void) = bar;
> fp();
> ```

================
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 external function call, use of ``longjmp``, or other means.
----------------
gulfem wrote:
> aaron.ballman wrote:
> > gulfem wrote:
> > > aaron.ballman wrote:
> > > > gulfem wrote:
> > > > > I think this property is transitive. 
> > > > > If a leaf function somehow enters into caller's translation unit (either via direct call or a via its call chain), it will violate the rule that says "Calls to external functions with this attribute must return to the current compilation unit only by return or by exception handling". 
> > > > > Entering via its call chain is not a return or an exception handling.
> > > > > Do you agree with that?
> > > > > I think this property is transitive. ... Do you agree with that?
> > > > 
> > > > That makes sense to me! I think I'd recommend a slight modification to the docs then:
> > > > 
> > > > ```
> > > > 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, possibly transitive, external function call, use of ``longjmp``, or other means.
> > > > ```
> > > > The last question is: by "direct" does the GCC docs imply that calls back to the caller's TU through a function *pointer* are not UB? I'd imagine those would also be bad, but I'd like to make sure (and perhaps we can remove the `direct` from the wording if calls through function pointers are disallowed).
> > > I think the idea is that the control-flow should never come back to the caller's translation unit by any kind of control-flow changing mechanism (direct/indirect and call/jump).
> > > The compiler might use this hint to do optimizations based on that assumption, so the user should ensure that.
> > > Could you please explain what do you mean by calling back via function pointer?
> > > I thought that callback function is basically calling back via a function pointer. 
> > > Could you please explain what do you mean by calling back via function pointer?
> > > I thought that callback function is basically calling back via a function pointer.
> > 
> > I had confused myself -- I think @jdoerfert answered my confusion in an earlier comment and I just wasn't thinking about that case hard enough. I think we want to drop the word "direct" from "external function call" because it doesn't matter whether the user says:
> > ```
> > void bar(void); // Assume calling this is bad
> > bar();
> > ```
> > and
> > ```
> > void bar(void); // Assume calling this is bad
> > void (*fp)(void) = bar;
> > fp();
> > ```
> > > Could you please explain what do you mean by calling back via function pointer?
> > > I thought that callback function is basically calling back via a function pointer.
> > 
> > I had confused myself -- I think @jdoerfert answered my confusion in an earlier comment and I just wasn't thinking about that case hard enough. I think we want to drop the word "direct" from "external function call" because it doesn't matter whether the user says:
> > ```
> > void bar(void); // Assume calling this is bad
> > bar();
> > ```
> > and
> > ```
> > void bar(void); // Assume calling this is bad
> > void (*fp)(void) = bar;
> > fp();
> > ```
> 
> 
I removed the direct from the documentation.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90275/new/

https://reviews.llvm.org/D90275