eddyz87 added a comment.
In D152986#4425736 <https://reviews.llvm.org/D152986#4425736>, @rnk wrote:
>
Thank you for the review!
> 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?
> The BPF backend and verifier needs to learn to tolerate valid LLVM transforms
> if it wants to be a real LLVM backend. Of course, you can do what you like.
Well, I agree with that. The issue here is with the map API design: it is
polymorphic, but it is expected that at load time verifier can replace all
polymorphic calls with static calls.
People know it and write code in a way that allows verifier to infer which
static functions to call. So I have limited options here:
- either adjust IR at early stages of pipeline so that specific calls are not
altered;
- or do nothing and recommend users to use `[[clang::nomerge]]` on statement
level or insert something like `asm volatile ("" :::"memory")` here and there.
But, yeah, I understand that is is not a C language semantics and headache is
mine.
> Considered in the context of the original use case, I think it's reasonable
> to allow the attribute on function pointers for the same reasons we allow it
> on function declarations. It makes it easy to work the attribute onto the
> direct call sites of the function without modifying tons of source code.
> However, I'd like to see clearer documentation on the limitations.
Please see my "inline" response.
================
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.
----------------
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
}
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152986/new/
https://reviews.llvm.org/D152986
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
