[PATCH] D135926: [clang] Fix crash with -funique-internal-linkage-names

2022-10-14 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram accepted this revision. tmsriram added a comment. This revision is now accepted and ready to land. LGTM, thanks for fixing this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135926/new/ https://reviews.llvm.org/D135926 __

[PATCH] D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation

2022-08-25 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram accepted this revision. tmsriram added inline comments. Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:589 + +if (SampleProfileHasFUnique) { + // If profile also uses funqiue, nothing to do here. Maybe rewrite this slightly as: // If S

[PATCH] D105226: [Clang] allow overriding -fbasic-block-sections

2021-06-30 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram accepted this revision. tmsriram added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105226/new/ https://reviews.llvm.org/D105226 ___

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

2021-03-18 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:18 +// since it doesn't come with valid prototype. +static int bar(a) int a; +{ Nice test, I didnt know you could do this! Repository: rG LLVM Github Monorepo

[PATCH] D98392: Disable Unique Internal Linkage Names for internal global vars

2021-03-11 Thread Sriraman Tallam via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcdb42a4cc423: Disable unique linkage suffixes ifor global vars until demanglers can be fixed. (authored by tmsriram). Herald added a project: clang.

[PATCH] D96109: Refactor implementation of -funique-internal-linkage-names.

2021-03-05 Thread Sriraman Tallam via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG78d0e91865f6: Refactor -funique-internal-linakge-names implementation. (authored by tmsriram). Herald added a project: clang. Herald added a subscrib

[PATCH] D94154: Unique Internal Linkage Name suffixes must be demangler friendly

2021-01-11 Thread Sriraman Tallam via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd8c6d24359f1: -funique-internal-linkage-names appends a hex md5hash suffix to the symbol name… (authored by tmsriram). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository:

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

2021-01-08 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D93747#2488387 , @dblaikie wrote: > Seems alright to me - I think we've hashed out the deeper issues (missing > opportunity for C functions which could/should be addressed by moving the > implementation to the frontend, where

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

2021-01-07 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram accepted this revision. tmsriram added a comment. This revision is now accepted and ready to land. In D92633#2485811 , @MaskRay wrote: > In D92633#2434768 , @tmsriram wrote: > >> In D92633#2434766

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

2021-01-06 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D93747#2482726 , @hoy wrote: > In D93747#2482519 , @tmsriram wrote: > >> In D93747#2481494 , @dblaikie wrote: >> >>> In D93747#2481383

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

2021-01-06 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D93747#2481494 , @dblaikie wrote: > In D93747#2481383 , @hoy wrote: > >> In D93747#2481304 , @tmsriram wrote: >> >>> In D93747#2481223

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

2021-01-05 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D93747#2481223 , @tmsriram wrote: > In D93747#2481163 , @dblaikie wrote: > >> In D93747#2481095 , @hoy wrote: >> >>> In D93747#2481073

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

2021-01-05 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D93747#2481163 , @dblaikie wrote: > In D93747#2481095 , @hoy wrote: > >> In D93747#2481073 , @dblaikie wrote: >> >>> In D93747#2481053

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

2021-01-05 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D93747#2480442 , @dblaikie wrote: >>> In D93747#2470178 , @tmsriram >>> wrote: >>> In D93747#2469556 , @hoy wrote: >> In D93656

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

2020-12-29 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. This change looks good to me but I would like @rnk to sign off. My opinion is that the change is simple enough. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93747/new/ https://reviews.llvm.org/D93747 __

[PATCH] D93082: Append ".__part." to all basic block section symbols.

2020-12-23 Thread Sriraman Tallam via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG34e70d722dfd: Append ".__part." to every basic block section symbol. (authored by tmsriram). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://revie

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

2020-12-23 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D93747#2470189 , @hoy wrote: > In D93747#2470178 , @tmsriram wrote: > >> In D93747#2469556 , @hoy wrote: >> In D93656

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

2020-12-23 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D93747#2469556 , @hoy wrote: >> In D93656 , @dblaikie wrote: >> Though the C case is interesting - it means you'll end up with C functions >> with the same DWARF 'name' but different linkage na

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

2020-12-22 Thread Sriraman Tallam via Phabricator via cfe-commits
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.

2020-12-22 Thread Sriraman Tallam via Phabricator via cfe-commits
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.

2020-12-21 Thread Sriraman Tallam via Phabricator via cfe-commits
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.

2020-12-21 Thread Sriraman Tallam via Phabricator via cfe-commits
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.

2020-12-21 Thread Sriraman Tallam via Phabricator via cfe-commits
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] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

2020-12-04 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D92633#2434766 , @MaskRay wrote: > In D92633#2434714 , @tmsriram wrote: > >> Correct me if I am wrong, but I do see that this behavior is touched. Line >> 10 in -fdirect-access-externa

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

2020-12-04 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D92633#2434511 , @MaskRay wrote: > In D92633#2434337 , @tmsriram wrote: > >> In D92633#2434267 , @MaskRay wrote: >> >>> In D92633#2434231

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

2020-12-04 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D92633#2434267 , @MaskRay wrote: > In D92633#2434231 , @tmsriram wrote: > >> Sorry just one more thing which is a bit concerning: >> >> When I do : >> >> $ clang -fPIC -frxternal-data-a

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

2020-12-04 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D92633#2434212 , @MaskRay wrote: > In D92633#2434166 , @tmsriram wrote: > >> LGTM. I checked it completely and the patch makes total sense to me. > > Thanks! The name bothers me a bit.

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

2020-12-04 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. Sorry just one more thing which is a bit concerning: When I do : $ clang -fPIC -frxternal-data-access foo.c You will omit the GOT but this object can go into a shared object and break the build as this does not apply to shared objects? Should we allow this at all f

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

2020-12-04 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. LGTM. I checked it completely and the patch makes total sense to me. Comment at: clang/include/clang/Driver/Options.td:1866 def fno_pie : Flag<["-"], "fno-pie">, Group; +def fdirect_access_external_data : Flag<["-"], "fdirect-access-external-data">,

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

2020-12-04 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D92633#2433979 , @MaskRay wrote: > In D92633#2433108 , @tmsriram wrote: > >> You said : "The name -mpie-copy-relocations is misleading [1] and does not >> capture the idea that this opt

[PATCH] D92633: Add -f[no-]direct-access-external-data to deprecate -mpie-copy-relocations

2020-12-03 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. You said : "The name -mpie-copy-relocations is misleading [1] and does not capture the idea that this option can actually apply to all of -fno-pic,-fpie, ..." Could you please clarify why this option needs to apply to -fno-pic? Here is what I tried with trunk clang:

[PATCH] D90815: -fbasic-block-sections=list=: Suppress output if failed to open the file

2020-11-06 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram accepted this revision. tmsriram added a comment. This revision is now accepted and ready to land. Thanks for fixing this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90815/new/ https://reviews.llvm.org/D90815 __

[PATCH] D89617: Prepend "uniq" to symbol names hash with -funique-internal-linkage-names

2020-10-26 Thread Sriraman Tallam via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGad1b9daa4bf4: Prepend "__uniq" to symbol names hash with -funique-internal-linkage-names. (authored by tmsriram). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LL

[PATCH] D89500: Fix the error message with -fbasic-block-sections=list=

2020-10-20 Thread Sriraman Tallam via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf88785460ecf: Improve file doesnt exist error with -fbasic-block-sections= (authored by tmsriram). Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87921: Fix -funique-internal-linkage-names to work with -O2 and new pass manager

2020-09-21 Thread Sriraman Tallam via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6950db36d33d: The wrong placement of add pass with optimizations led to -funique-internal… (authored by tmsriram). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to c

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-09 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram accepted this revision. tmsriram added a comment. This revision is now accepted and ready to land. This LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87047/new/ https://reviews.llvm.org/D87047 ___

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-02 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D85408#2253055 , @MaskRay wrote: > I am still reading the patch, but I have noticed one thing worth discussing. > `.bb_addr_map` is a non-SHF_ALLOC section (meaning that it is not part of the > memory image). An absolute relo

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-06-02 Thread Sriraman Tallam via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. tmsriram marked 2 inline comments as done. Closed by commit rGe0bca46b0854: Options for Basic Block Sections, enabled in D68063 and D73674. (authored by tmsriram). Changed prior to commit: https://reviews.llvm.org/D68049?

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-06-01 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram marked an inline comment as done. tmsriram added a comment. In D68049#2066937 , @MaskRay wrote: > LLD side changes look good. Are you waiting on an explicit approval from > @rmisth ? Yes. Comment at: lld/ELF/LTO.cpp:79 //

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-06-01 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-05-26 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-05-18 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:1336 + +Generate labels for each basic block or place each basic block or a subset of basic blocks in its own section + rsmith wrote: > This file is automatically generated from

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-05-18 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 264733. tmsriram marked 4 inline comments as done. tmsriram added a comment. Address reviewer comments: - New diagnostic kind - Document list= - Add docbrief CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-05-18 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. Ping. Comment at: clang/include/clang/Basic/CodeGenOptions.h:114-127 + // -fbasic-block-sections=. The allowed values with this option are: + // {"labels", "all", "", "none"}. + // + // "labels": Only generate basic block symbols (labels) for

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-05-12 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 263371. tmsriram marked 5 inline comments as done and an inline comment as not done. tmsriram added a comment. Address reviewer comments: 1. Use Diagnostic instead of errs 2. Move test input to Inputs/ 3. Fix option description. CHANGES SINCE LAST ACTION

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-05-11 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram marked an inline comment as done. tmsriram added inline comments. Comment at: lld/ELF/LTO.cpp:80 // Check if basic block sections must be used. // Allowed values for --lto-basicblock-sections are "all", "labels", // "", or none. This is the equivalent -

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-05-11 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 263322. tmsriram added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Files: clang/docs/ClangCommandLineReference.rst clang/docs/UsersManual.rst clang/include/clang/Basic/CodeGenOp

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

2020-05-07 Thread Sriraman Tallam via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe8147ad82226: Uniuqe Names for Internal Linkage Symbols. (authored by tmsriram). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

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

2020-05-07 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 262797. tmsriram added a comment. Update patch with changes to BackendUtil.cpp (forgot this file). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 Files: clang/docs/UsersManual.rst clang/include/clang/B

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

2020-05-07 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 262791. tmsriram added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 Files: clang/docs/UsersManual.rst clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Driver/Opti

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-05-05 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. @rsmith Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

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

2020-05-04 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. @rnk Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-04-29 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.l

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

2020-04-27 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-04-23 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:486-501 + Options.BBSections = + llvm::StringSwitch(CodeGenOpts.BBSections) + .Case("all", llvm::BasicBlockSection::All) + .Case("labels", llvm::BasicBlockSection::Labels) +

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-04-23 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 259626. tmsriram marked 5 inline comments as done. tmsriram added a comment. Herald added subscribers: dexonsmith, steven_wu, hiraditya, arichardson, emaste. Herald added a reviewer: espindola. Document the flags, rename the options. CHANGES SINCE LAST ACTI

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-04-21 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram marked an inline comment as done. tmsriram added inline comments. Comment at: clang/include/clang/Driver/Options.td:1975 +def fbasicblock_sections_EQ : Joined<["-"], "fbasicblock-sections=">, Group, Flags<[CC1Option, CC1AsOption]>, + HelpText<"Place each function's bas

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

2020-04-21 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram marked 3 inline comments as done. tmsriram added a comment. @rnk Good now? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

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

2020-04-17 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D73307#1989924 , @erichkeane wrote: > In D73307#1989740 , @rnk wrote: > > > In D73307#1978140 , @tmsriram > > wrote: > > > > > In D73307#1972388

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

2020-04-15 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 257853. tmsriram added a comment. Updated this patch, using a pass to convert symbol names in D78243 Also, removed the test for -fmacro-prefix-map. For -ffile-prefix-map, looks like getSourceFileName() should be modified

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-04-14 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-04-13 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram marked 2 inline comments as done. tmsriram added a comment. In D73307#1972388 , @rnk wrote: > Regarding the alias attribute, it occurs to me that reimplementing this as an > early LLVM pass would not have that problem. Do you think that would be

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-04-09 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D68049#1972297 , @dblaikie wrote: > In D68049#1971276 , @MaskRay wrote: > > > In D68049#1970825 , @tmsriram > > wrote: > > > > > Ping. > > > > >

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

2020-04-08 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. We just noticed an issue with alias attribute and this option. Here is the code that exposes the problem: alias_global.c static int foo; extern int bar __attribute__((alias("foo"))) $ clang -c alias_global.c -funique-internal-linkage-names alias_global.c:4:31: error:

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-04-08 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-04-07 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added inline comments. Comment at: clang/docs/UsersManual.rst:1684 + linkage symbols. The unique name is obtained by appending the hash of the + full module name to the original symbol. This option is particularly useful + in attributing profile information to the

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

2020-04-07 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 255871. tmsriram marked 4 inline comments as done. tmsriram added a comment. Change description and handle -ffile-prefix-map/-fmacro-prefix-map. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 Files: clan

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

2020-04-06 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-04-03 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added inline comments. Comment at: clang/test/CodeGen/basicblock-sections.c:35 +// +// BB_WORLD: .section .text.world,"ax",@progbits +// BB_WORLD: world MaskRay wrote: > I haven't read through the previous threads whether we should use a .c -> .s > test

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-04-03 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 254999. tmsriram marked 4 inline comments as done. tmsriram added a comment. Fix test and make error check at driver. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Files: clang/include/clang/Basic/CodeG

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

2020-04-01 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 254347. tmsriram added a comment. Rebase and add tests for anonymous namespace symbol and function static global. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 Files: clang/docs/UsersManual.rst clang/

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

2020-03-30 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram marked an inline comment as done. tmsriram added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:5 +// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm -funique-internal-linkage-names -o - < %s | FileCheck %s --check-prefix=UNIQUE +

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-03-27 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 253271. tmsriram marked 5 inline comments as done. tmsriram added a reviewer: eli.friedman. tmsriram added a comment. Clang options for Basic Block Sections enabled in D68063 and D73674 1.

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

2020-03-25 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D73307#1942805 , @MaskRay wrote: > In D73307#1932131 , @rnk wrote: > > > At a higher level, should this just be an IR pass that clang adds into the > > pipeline when the flag is set? It

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

2020-03-25 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 252688. tmsriram marked 4 inline comments as done. tmsriram added a comment. Changes to description of flag, remove redundant check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 Files: clang/docs/Users

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

2020-03-23 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-03-21 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1125 const auto *ND = cast(GD.getDecl()); std::string MangledName = getMangledNameImpl(*this, GD, ND); rnk wrote: > There are several other callers of getMangledNameImpl. Shou

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

2020-03-21 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 251868. tmsriram marked 11 inline comments as done. tmsriram added a comment. Address @rnk comments: - Moved implementation to getMangledNameImpl to catch multi-versioned symbols and internal linkage globals - Moved hash computation to consrtuctor - Renamed

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

2020-03-19 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram marked 4 inline comments as done. tmsriram added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1135 +llvm::MD5 Md5; +Md5.update(getModule().getSourceFileName()); +llvm::MD5::MD5Result R; hubert.reinterpretcast wrote: > davi

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

2020-03-19 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 251469. tmsriram marked 4 inline comments as done. tmsriram added a comment. Address reviewer comments. - reword comment - rewrite test to use -emit-llvm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 Fil

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

2020-03-19 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added inline comments. Comment at: clang/test/CodeGen/unique-internal-funcnames.c:16 +// UNIQUE-NOT: foo: +// UNIQUE: foo.{{[0-9a-f]+}}: lebedev.ri wrote: > What does `getModule().getSourceFileName()` contain? > The full path to the source file, or just

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

2020-03-19 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram marked an inline comment as done. tmsriram 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

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

2020-03-19 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 251409. tmsriram marked 2 inline comments as done. tmsriram added a comment. Address reviewer comments. Fix test and delete blank line. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 Files: clang/includ

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

2020-03-18 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp: + !getModule().getSourceFileName().empty()) { +llvm::MD5 Md5; +Md5.update(getModule().getSourceFileName()); mtrofin wrote: > Just a thought: md5 is a non-bijective t

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

2020-03-18 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 251220. tmsriram marked 5 inline comments as done. tmsriram added a comment. Address Reviewer comments: - Add new driver test. - Fix existing test to only check for code output. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://revi

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-02-11 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. >>> I think the patch series should probably be structured this way: >>> >>> 1. LLVM CodeGen: enables basic block sections. >>> 2. clang Driver/Frontend/CodeGen: pass basic block sections options to LLVM. >>> 3. LLVM CodeGen: which enables the rest of Propeller options.

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-02-11 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D68049#1870094 , @tmsriram wrote: > In D68049#1868623 , @MaskRay wrote: > > > > In D68049#1865967 , @MaskRay > > > wrote: > > > If you don't mi

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-02-11 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 243947. tmsriram added a comment. Remove usage of "propeller". Fix header inclusion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/cla

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-02-11 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D68049#1870094 , @tmsriram wrote: > In D68049#1868623 , @MaskRay wrote: > > > > In D68049#1865967 , @MaskRay > > > wrote: > > > If you don't mi

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-02-11 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D68049#1868623 , @MaskRay wrote: > > In D68049#1865967 , @MaskRay wrote: > > If you don't mind, I can push a Diff to this Differential which will > > address these review comments. > >

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-02-10 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 243614. tmsriram marked 3 inline comments as done. tmsriram added a comment. Removed getBBSectionsList (moved to LLVM) and address other reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Fi

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-02-10 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram added a comment. In D68049#1865967 , @MaskRay wrote: > If you don't mind, I can push a Diff to this Differential which will address > these review comments. Let me update this patch asap as we refactored getBBSectionsList into llvm as it is sh

[PATCH] D68049: Propeller: Clang options for basic block sections

2020-02-03 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 242196. tmsriram added a comment. Splitting the clang patch into several pieces: 1. This is the parent patch and just contains support for basic block section options. 2. A separate patch for unique internal function names 3. A separate patch for an option

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

2020-01-24 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 240312. tmsriram added a comment. Minor changes. Remove CC1option on "-fno", Remove "$" from unique suffix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 Files: clang/include/clang/Basic/CodeGenOptions

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

2020-01-24 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram marked an inline comment as done. tmsriram added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1117 +llvm::MD5::stringifyResult(R, Str); +std::string UniqueSuffix = (".$" + Str).str(); +MangledName = MangledName + UniqueSuffix;

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

2020-01-24 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 240253. tmsriram added a comment. Remove an unnecessary header file inclusion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Dri

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

2020-01-24 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram updated this revision to Diff 240249. tmsriram added a comment. Use Hash of Module's source file name directly. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/

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

2020-01-24 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram marked an inline comment as done. tmsriram added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:966 OPT_fno_unique_section_names, true); + Opts.UniqueInternalFuncNames = Args.hasFlag( + OPT_funique_i

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

2020-01-24 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram marked an inline comment as done. tmsriram added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp: + this->getFunctionLinkage(GD) == llvm::GlobalValue::InternalLinkage) { +std::string UniqueSuffix = getUniqueModuleId(&getModule(), true); +

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

2020-01-23 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram created this revision. tmsriram added reviewers: MaskRay, mehdi_amini, davidxl, pcc, rnk. tmsriram added a parent revision: D68049: Propeller: Clang options for basic block sections . This is a subset of the functionality in Propeller patches. This patch adds a new clang option, -funiq

  1   2   >