[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D73307#1942838 <https://reviews.llvm.org/D73307#1942838>, @tmsriram wrote:

> In D73307#1942805 <https://reviews.llvm.org/D73307#1942805>, @MaskRay wrote:
>
> > In D73307#1932131 <https://reviews.llvm.org/D73307#1932131>, @rnk wrote:
> >
> > > At a higher level, should this just be an IR pass that clang adds into the pipeline when the flag is set? It should be safe to rename internal functions and give private functions internal linkage. It would be less invasive to clang and have better separation of concerns. As written, this is based on the source filename on the module, which is accessible from IR. The only reason I can think of against this is that the debug info might refer to the function linkage name, but maybe that is calculated later during codegen.
> >
> >
> > I second this suggestion. `clang/lib/CodeGen/BackendUtil.cpp` `EmitAssemblyHelper::EmitAssemblyWithNewPassManager` ModulePassManager may be a more appropriate place for this feature. There are examples from both sides:
>
>
> @rnk getMangledNameImpl for example adds multi-versioning suffixes,  "__regcall3_" and  "__device_stub".  Adding the hash is just so simple.  Is a pass really needed?  If so, I should parse ".<target>" suffixed Multi-versioining names and insert the hash in between as the demangling looks better?
>
> > 
> > 
> > - clang frontend: `#pragma redefine_extname`
> > - middle end IR->IR: the old pass manager has a feature -frewrite-map-file, which does a similar thing but is implemented as an IR transformation.


There may be some other interactions @rnk noticed. I am happy with either way.



================
Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:5
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm -funique-internal-linkage-names -o - < %s | FileCheck %s --check-prefix=UNIQUE
+
+static int glob;
----------------
Might be worth adding a few other internal linkage names.

* an object in an unnamed namespace
* an extern "C" static function
* a function-local static variable
* `label: &&label`

Hope @mtrofin and @davidxl can clarify what internal names may benefit AFDO and we can add such internal names specifically.


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

https://reviews.llvm.org/D73307