[PATCH] D22600: [PGO] Fix profile mismatch in Comdat function with pre-inliner

[Rong Xu via llvm-commits]

xur marked an inline comment as done.

================
Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:393
@@ +392,3 @@
+      // For aliases, change the name directly.
+      GA->setName(Twine(GA->getName() + "." + Twine(FunctionHash)));
+      continue;
----------------
davidxl wrote:
> Add assert that the alias target is still F
I'll assert GA's comdat is the same as OrigComdat.

================
Comment at: test/Transforms/PGOProfile/comdat_rename.ll:2
@@ +1,3 @@
+; RUN: opt < %s -pgo-instr-gen -S | FileCheck %s
+; RUN: opt < %s -passes=pgo-instr-gen -S | FileCheck %s
+
----------------
davidxl wrote:
> you may want to try this test case on other platform such as coff to make sure it works:
> 
>  -mtriple=x86_64-pc-win32-coff 
The test failed for the AvailableExternallyLinkage case. The reason is in needsComdatForCounter().

For AvailableExternallyLinkage, since this is not ELF format target, we return false.

I'm wondering if we should move the checking for TargetTriple() after the linkage check. The target can have Comdat support for nonELF targets after all.



https://reviews.llvm.org/D22600