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

[David Li via Phabricator via cfe-commits]

davidxl added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1129
+  // should get unique names.  Use the hash of module name to get a unique
+  // identifier and this is a best effort.
+  if (getCodeGenOpts().UniqueInternalFuncNames &&
----------------
lebedev.ri wrote:
> maybe
> `identifier as this is a best-effort solution`
> 
Address review comment here.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1135
+    llvm::MD5 Md5;
+    Md5.update(getModule().getSourceFileName());
+    llvm::MD5::MD5Result R;
----------------
Source filenames are not guaranteed to be unique, or it does contain the path as well?


================
Comment at: clang/test/CodeGen/unique-internal-funcnames.c:3
+
+// RUN: %clang -target x86_64 -S -o - %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang -target x86_64 -S -funique-internal-funcnames -o -  %s | FileCheck %s --check-prefix=UNIQUE
----------------
MaskRay wrote:
> You can hardly find any .c -> .s test in clang/test. We mostly do .c -> .ll testing. `.ll` -> `.s` are in llvm/test/CodeGen. 
Is this convention documented somewhere? Any concerns of producing .s?


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

https://reviews.llvm.org/D73307