[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

Tue Dec 22 13:05:18 PST 2020

hoy added a comment.

In D93656#2468821 <https://reviews.llvm.org/D93656#2468821>, @aeubanks wrote:

> Also it looks like this is doing 2 different things, the moving of things from Clang to LLVM's PassBuilder, and separately the change to the pass itself. Can these be separated?

I'm not sure about a good way to separate them. There are Clang tests that may fail with removing the pass from clang while not adding it correspondingly in llvm. Adding the pass in llvm while not removing it from Clang may cause the pass to run twice which may also fail the Clang tests. What do you think?

================
Comment at: llvm/include/llvm/Passes/PassBuilder.h:140
   bool DebugLogging;
+  bool UniqueLinkageNames;
   TargetMachine *TM;
----------------
aeubanks wrote:
> This seems better suited to be part of PipelineTuningOptions rather than directly in PassBuilder
Sounds good. Will move it into PipelineTuningOptions.

================
Comment at: llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp:26-29
+static cl::opt<bool> UniqueDebugAndProfileNames(
+    "unqiue-debug-profile-names", cl::init(true), cl::Hidden,
+    cl::desc("Uniqueify debug and profile symbol Names"));
+
----------------
tmsriram wrote:
> hoy wrote:
> > dblaikie wrote:
> > > Do you have plans to use the flag in testing, etc? If not, I guess you might as well bake in the behavior you want if no one else is using this/there's no current use case for the flexibility?
> > I'm not quite sure if updating debug names are required in all use cases. @tmsriram do you think we can just remove the flag?
> I vote for removing it.
Sounds good, will remove it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93656