[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

Thu Jun 17 14:59:40 PDT 2021

melver added a comment.

In D104475#2825711 <https://reviews.llvm.org/D104475#2825711>, @MaskRay wrote:

> In D104475#2825666 <https://reviews.llvm.org/D104475#2825666>, @melver wrote:
>
>> In D104475#2825297 <https://reviews.llvm.org/D104475#2825297>, @MaskRay wrote:
>>
>>> Should `no_profile` specify the inlining behavior? Though `no_sanitize_*` don't specify that, either.
>>
>> I think it's somehow implicit, but I can't quite say how. There are some tests that check this, e.g.
>> compiler-rt/test/asan/TestCases/inline.cpp
>> [...]
>> noinstr does add noinline, but other uses of the attribute alone might not. But in the end inlining is a red herring, it just seems to be the easiest way to enforce the promised contract.  See https://lore.kernel.org/lkml/CANpmjNNRz5OVKb6PE7K6GjfoGbht_ZhyPkNG9aD+KjNDzK7hGg@mail.gmail.com/
>> [...]
>
> Thanks for the comments.  Recently there is a discussion about LLVM IR function attribute `nointeropt` (similar to a debugging-only GCC function attribute `noipa`).
> People tend to think attributes should be orthogonal. I think not suppressing inlining for the IR attribute `no_profile` is nice.
> Ideally the GNU function attribute (C/C++) does not diverge much from the IR semantics.
> So if we don't want to offer guarantee for IR/C/C++ attributes, we can document that users may need to additionally specify `__attribute__((noinline))` to suppress inlining.

As long as it remains practical. I don't want us ending up with a mess of an attribute like we once had for KASAN: __no_kasan_or_inline -- because 'static inline __no_sanitize_address foo(void) { ... }' did the wrong thing, because a) attribute was incompatible with 'inline', and b) caused instrumentation if it was inlined (AFAIK that behaviour is fixed and was a GCC-only bug).

Essentially, as long as no_foo means that code in function where no_foo is specified is never instrumented by foo, then we're good. If that code is inlined or not doesn't matter, as long as the contract is honored. If that's not the case we have a problem because we need ugly workarounds for what are essentially compiler bugs (contract not honored).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104475