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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.


================
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
----------------
rnk wrote:
> tmsriram wrote:
> > davidxl wrote:
> > > 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?
> > There are more than 200 tests in clang that do -S from .c -> .s and tens of tests in CodeGen alone.  I can change this to .ll but curious where this "hardly any" comes from?
> I can't specifically find documentation for this convention, but it is generally understood that every feature should have targeted unit tests that exercise the minimum amount of functionality. In this case, that means using `%clang_cc1 -emit-llvm -triple NNN` and checking the IR.
Not that many -S .c -> .s tests (`rg '%clang.* -S ' | egrep -v '[-]###|emit-llvm|-verify' | awk -F: '{print $1}' | sort -u | wc -l`). Many are about -emit-llvm -### -verify or misused -v. They do not need to produce assembly.


================
Comment at: clang/test/Driver/funique-internal-funcnames.c:4
+// CHECK-OPT: "-funique-internal-funcnames"
+// CHECK-NOOPT-NOT: {{-funique-internal-funcnames}}
----------------
This negative pattern needs to be changed as well.


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

https://reviews.llvm.org/D73307