tmsriram added a comment.

https://reviews.llvm.org/D73307#1932131  rnk@ mentioned this :  "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 just wanted to mention it  here that this was anticipated and was missed in 
the original patch, my bad as I didnt think about DebugInfo change.  However, I 
think it is pretty straightforward to change the linkage name so I would still 
keep the pass approach.



================
Comment at: llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp:27
+static cl::opt<bool> UniqueDebugAndProfileNames(
+    "unqiue-debug-profile-names", cl::init(false), cl::Hidden,
+    cl::desc("Uniqueify debug and profile symbol Names"));
----------------
Can we make it true by default?  Atleast the DW_AT_linkage_name must reflect 
the new name by default IMO.


================
Comment at: llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp:51
+          auto Name = MDB.createString(F.getName());
+          SP->replaceRawLinkageName(Name);
+        }
----------------
Do we need to check if it had a rawLinkageName before replacing it?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93656

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to