[PATCH] D152986: [clang] Allow 'nomerge' attribute for function pointers

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

In D152986#4426256 <https://reviews.llvm.org/D152986#4426256>, @eddyz87 wrote:

> In D152986#4425736 <https://reviews.llvm.org/D152986#4425736>, @rnk wrote:
>
>> The purpose of the attribute is really limited to preserving source location information on instructions, and this isn't really a supported usage.
>
> But it would prevent merging, right? I understand that using this attribute might block some legitimate optimizations.
> Or do you mean that it is a "best effort" thing and not all transformations might honor it?

Essentially, yes.

Anyway, I think this looks good, please apply the doc update.



================
Comment at: clang/include/clang/Basic/AttrDocs.td:551-555
 calls to the specified function from merging. It has no effect on indirect 
 calls.
+
+``nomerge`` attribute can be specified for pointers to functions, all
+calls done through such pointer would be protected from merging.
----------------
eddyz87 wrote:
> rnk wrote:
> > This statement of the attribute having "no effect on indirect calls" is slightly confusing now that we talk about function pointers immediately afterward. Can you please rework this a bit, and clarify that when applied to function pointers, the attribute only takes effect when the call target is directly the variable which carries the attribute? For example, this has no effect:
> > ```
> > void (*fp)() __attribute__((nomerge));
> > void callit() {
> >   auto tmp = fp;
> >   tmp();
> >   (*fp)(); // I think TargetDecl will be null in the code, tell me if I'm wrong
> > }
> > ```
> I can make it more elaborate, would the text as below be fine?
> Regarding TargetDecl value it is not null both times:
> - `(VarDecl 'tmp' (ImplicitCastExpr (DeclRefExpr (Var fp))))`
> - `(VarDecl 'fp')`
> 
> ---
> 
> ``nomerge`` attribute can also be used as function attribute to prevent all 
> calls to the specified function from merging. It has no effect on indirect 
> calls to such functions. For example:
> 
> .. code-block:: c++
> 
>   [[clang::nomerge]] void foo(int) {}
>   
>   void bar(int x) {
>     auto *ptr = foo;
>     if (x) foo(1); else foo(2); // will not be merged
>     if (x) ptr(1); else ptr(2); // indirect call, can be merged
>   }
> 
> ``nomerge`` attribute can also be used for pointers to functions to
> prevent calls through such pointer from merging. In such case the
> effect applies only to a specific function pointer. For example:
> 
> .. code-block:: c++
> 
>   [[clang::nomerge]] void (*foo)(int);
>   
>   void bar(int x) {
>     auto *ptr = foo;
>     if (x) foo(1); else foo(2); // will not be merged
>     if (x) ptr(1); else ptr(2); // 'ptr' has no 'nomerge' attribute,
>                                 // can be merged
>   }
> 
Looks good to me, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152986