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

This revision was automatically updated to reflect the committed changes. Closed by commit rG01f0d162d672: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines. (authored by hoy). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93656/new/

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

hoy updated this revision to Diff 314257. hoy added a comment. Addressing comments from dblaikie. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93656/new/ https://reviews.llvm.org/D93656 Files: llvm/include/llvm/Passes/PassBuilder.h llvm/lib/P

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

hoy added a comment. In D93656#2475856 , @dblaikie wrote: > Looks good - test cases might benefit from some descriptive comments > (explaining why the pseudo probe pass needs to be enabled to test the unique > linkage name pass - I guess to check that it

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

dblaikie accepted this revision. dblaikie added a comment. Looks good - test cases might benefit from some descriptive comments (explaining why the pseudo probe pass needs to be enabled to test the unique linkage name pass - I guess to check that it appears in just the right place in the pass p

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

hoy updated this revision to Diff 314200. hoy marked an inline comment as done. hoy added a comment. Adding a test for the new pseudo probe test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93656/new/ https://reviews.llvm.org/D93656 Files: llv

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

hoy marked an inline comment as done. hoy added inline comments. Comment at: llvm/tools/opt/NewPMDriver.cpp:136-138 +static cl::opt PseudoProbeForProfiling( +"new-pm-pseudo-probe-for-profiling", cl::init(false), cl::Hidden, +cl::desc("Emit pseudo probes to enable PGO prof

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

dblaikie added inline comments. Comment at: llvm/tools/opt/NewPMDriver.cpp:136-138 +static cl::opt PseudoProbeForProfiling( +"new-pm-pseudo-probe-for-profiling", cl::init(false), cl::Hidden, +cl::desc("Emit pseudo probes to enable PGO profile generation.")); -

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

aeubanks added inline comments. Comment at: llvm/tools/opt/NewPMDriver.cpp:136-138 +static cl::opt PseudoProbeForProfiling( +"new-pm-pseudo-probe-for-profiling", cl::init(false), cl::Hidden, +cl::desc("Emit pseudo probes to enable PGO profile generation.")); -

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

hoy updated this revision to Diff 314043. hoy added a comment. Removing the checks of VerifierAnalysis in test code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93656/new/ https://reviews.llvm.org/D93656 Files: llvm/include/llvm/Passes/PassBui

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

hoy added inline comments. Comment at: llvm/tools/opt/NewPMDriver.cpp:136-138 +static cl::opt PseudoProbeForProfiling( +"new-pm-pseudo-probe-for-profiling", cl::init(false), cl::Hidden, +cl::desc("Emit pseudo probes to enable PGO profile generation.")); d

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

dblaikie added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global hoy wrote: > d

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

hoy added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global dblaikie wrote: > h

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

dblaikie added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global hoy wrote: > d

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

aeubanks accepted this revision. aeubanks added a comment. This revision is now accepted and ready to land. LGTM with nit, but somebody else more familiar with this should LGTM to make sure this makes sense Comment at: llvm/test/Transforms/UniqueLinkageNames/unique-internal-l

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

hoy updated this revision to Diff 313650. hoy marked an inline comment as done. hoy added a comment. Removing unnecessary test code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93656/new/ https://reviews.llvm.org/D93656 Files: llvm/include/llv

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

hoy marked an inline comment as done. hoy added inline comments. Comment at: llvm/test/Transforms/UniqueLinkageNames/unique-internal-linkage-names.ll:20 + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, enums: !2) +!1 = !DIFile(filename: "test.c", directory: "") -

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

aeubanks added inline comments. Comment at: llvm/test/Transforms/UniqueLinkageNames/unique-internal-linkage-names.ll:20 + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, enums: !2) +!1 = !DIFile(filename: "test.c", directory: "") is the debug info

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

hoy updated this revision to Diff 313630. hoy added a comment. Adding PTO checks in LLVM test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93656/new/ https://reviews.llvm.org/D93656 Files: llvm/include/llvm/Passes/PassBuilder.h llvm/lib/Pass

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

hoy added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global dblaikie wrote: > a

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

dblaikie added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global aeubanks wrote

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

aeubanks added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global dblaikie wrote

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

dblaikie added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global hoy wrote: > a

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

hoy added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global aeubanks wrote: > h

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

aeubanks added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global hoy wrote: > a

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

hoy added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global aeubanks wrote: > d

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

aeubanks added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global dblaikie wrote

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

dblaikie added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global hoy wrote: > d

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

hoy added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global dblaikie wrote: > D

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

dblaikie added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49 +// LPIPELINE: Unique Internal Linkage Names +// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass // PLAIN: @_ZL4glob = internal global Does this test

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

hoy updated this revision to Diff 313566. hoy edited the summary of this revision. hoy added a comment. Checking pipeline in clang test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93656/new/ https://reviews.llvm.org/D93656 Files: clang/lib/Co

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

hoy added a comment. In D93656#2469504 , @dblaikie wrote: > In D93656#2469485 , @hoy wrote: > >> In D93656#2468841 , @aeubanks wrote: >> >>> In D93656#2468834

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

hoy added a comment. The changes that rename debug linkage name has been separated as D93747 . I'm moving the discussion there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93656/new/ https://reviews.llvm.org/D936

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

hoy updated this revision to Diff 313488. hoy added a comment. Fixing IR test failure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93656/new/ https://reviews.llvm.org/D93656 Files: clang/lib/CodeGen/BackendUtil.cpp llvm/include/llvm/Passes/P

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

dblaikie added a comment. In D93656#2469485 , @hoy wrote: > In D93656#2468841 , @aeubanks wrote: > >> In D93656#2468834 , @hoy wrote: >> >>> In D93656#2468821

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

hoy updated this revision to Diff 313487. hoy added a comment. Separated PassBuilder changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93656/new/ https://reviews.llvm.org/D93656 Files: clang/lib/CodeGen/BackendUtil.cpp llvm/include/llvm/P

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

hoy added a comment. In D93656#2468841 , @aeubanks wrote: > In D93656#2468834 , @hoy wrote: > >> In D93656#2468821 , @aeubanks wrote: >> >>> Also it looks like this is doing

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

dblaikie added inline comments. Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2056-2059 + void replaceRawLinkageName(MDString *LinkageName) { +replaceOperandWith(3, LinkageName); + } + hoy wrote: > tmsriram wrote: > > dblaikie wrote: > > > The need t

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

aeubanks added a comment. In D93656#2468834 , @hoy wrote: > In D93656#2468821 , @aeubanks wrote: > >> Also it looks like this is doing 2 different things, the moving of things >> from Clang to LLVM's PassBuilder, a

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

hoy added a comment. In 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 sur

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

aeubanks added a comment. 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? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

aeubanks added inline comments. Comment at: llvm/include/llvm/Passes/PassBuilder.h:140 bool DebugLogging; + bool UniqueLinkageNames; TargetMachine *TM; This seems better suited to be part of PipelineTuningOptions rather than directly in PassBuilder Repo

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

tmsriram added inline comments. Comment at: llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp:26-29 +static cl::opt UniqueDebugAndProfileNames( +"unqiue-debug-profile-names", cl::init(true), cl::Hidden, +cl::desc("Uniqueify debug and profile symbol Names")); +

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

hoy added inline comments. Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2056-2059 + void replaceRawLinkageName(MDString *LinkageName) { +replaceOperandWith(3, LinkageName); + } + tmsriram wrote: > dblaikie wrote: > > The need to add this API makes m

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

tmsriram added inline comments. Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2056-2059 + void replaceRawLinkageName(MDString *LinkageName) { +replaceOperandWith(3, LinkageName); + } + dblaikie wrote: > The need to add this API makes me a bit uncerta

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

dblaikie added a subscriber: rnk. dblaikie added a comment. This should have a LLVM test coverage for the LLVM changes. (I realize they're also tested by the Clang test, because there's no way to test Clang's pass manager creation short of testing the effect of running the pass manager (hmm - /

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

hoy updated this revision to Diff 313187. hoy marked an inline comment as done. hoy added a comment. Addressing feedbacks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93656/new/ https://reviews.llvm.org/D93656 Files: clang/lib/CodeGen/BackendU

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

hoy marked 2 inline comments as done. hoy added inline comments. Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:881 +void DISubprogram::replaceRawLinkageName(MDString *LinkageName) { + replaceOperandWith(3, LinkageName); +} tmsriram wrote: > Nit, Move the body to

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

tmsriram added inline comments. Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:881 +void DISubprogram::replaceRawLinkageName(MDString *LinkageName) { + replaceOperandWith(3, LinkageName); +} Nit, Move the body to DebugInfoMetadata.h itself? Co

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

hoy added a comment. In D93656#2466689 , @tmsriram wrote: > 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 t

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

hoy added inline comments. Comment at: llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp:27 +static cl::opt UniqueDebugAndProfileNames( +"unqiue-debug-profile-names", cl::init(false), cl::Hidden, +cl::desc("Uniqueify debug and profile symbol Names")); -

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

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 wou

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

tmsriram added a comment. I too was currently working on changing the raw linkage names, DW_AT_linkage_name, and was about to send out a patch along these lines :). Thanks for doing this! I will take a look at it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

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

hoy created this revision. Herald added subscribers: dexonsmith, wenlei, hiraditya. hoy requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. `UniqueInternalLinkageNamesPass` is useful to CSSPGO, especially when pseudo probe is