[PATCH] D92800: [Clang] Make nomerge attribute a function attribute as well as a statement attribute.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: clang/test/CodeGen/attr-nomerge.cpp:8
+  [[clang::nomerge]] void f();
+  [[clang::nomerge]] virtual void g();
+  [[clang::nomerge]] static void f1();
----------------
zequanwu wrote:
> rnk wrote:
> > Hm, virtual functions, there's something worth thinking about. In this case, the attribute must appear on the call site if we want it to work as the user expects. I don't see the indirect call site for the virtual call in the IR below. I think it's worth testing for that. Consider taking `A*` and `B*` parameters to defeat any frontend devirtualization optimizations.
> If devirtualization failed, then only statement attribute would work.
Fair enough, that is similar to how `not_tail_called` works. Can you also update AttrDocs.td to mention the limitation?


================
Comment at: clang/test/CodeGen/attr-nomerge.cpp:17
+
+[[clang::nomerge]] bool bar() {
+  return true;
----------------
zequanwu wrote:
> rnk wrote:
> > I would leave this declaration as it was, so that all the statement attribute test cases you wrote below remain as they were, and the attribute continues to appear at the call site.
> > 
> > The `f` declaration below gives you coverage of the free function declaration case.
> I added `[[clang::nomerge]]` to `bar` in order to test that the generated IR should attach `nomerge` to call-sites which callee function already has this attribute.
Well, from the tests below, it seems like the attribute is *not* present on the call sites. Is that what you intended?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92800