[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

tmsriram created this revision. New clang option -fno-plt which avoids the PLT and lazy binding while making external calls. GCC supports -fno-plt, https://gcc.gnu.org/ml/gcc-patches/2015-05/msg1.html. This patch adds this to clang which marks all externally defined functions with the

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

tmsriram updated this revision to Diff 119950. tmsriram added a comment. Added test test/CodeGen/noplt.c https://reviews.llvm.org/D39079 Files: include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def lib/CodeGen/CGCall.cpp lib/Driver/ToolChains/Clang.cpp

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

tmsriram added inline comments. Comment at: lib/CodeGen/CGCall.cpp:1859 +if (auto *Fn = dyn_cast(TargetDecl)) { + if (!Fn->isDefined() && !AttrOnCallSite) { +FuncAttrs.addAttribute(llvm::Attribute::NonLazyBind); rnk wrote: > Remind me what

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

tmsriram added a comment. Ping. https://reviews.llvm.org/D39079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39759: Remove x86 specific code from noplt.c

tmsriram created this revision. https://reviews.llvm.org/D39079 breaks noplt.c for non-x86 targets. Remove x86 specific code from noplt.c https://reviews.llvm.org/D39759 Files: test/CodeGen/noplt.c Index: test/CodeGen/noplt.c

[PATCH] D39759: Remove x86 specific code from noplt.c

This revision was automatically updated to reflect the committed changes. Closed by commit rL317627: Change noplt.c to work for non-x86 targets. (authored by tmsriram). Changed prior to commit: https://reviews.llvm.org/D39759?vs=121977=121987#toc Repository: rL LLVM

[PATCH] D39759: Remove x86 specific code from noplt.c

tmsriram added a comment. More information here. This breaks non-x86 builds like PPC because: /home/buildbots/ppc64be-clang-test/clang-ppc64be/llvm/tools/clang/test/CodeGen/noplt.c:4:22: error: expected string not found in input // CHECK-NOPLT-NEXT: declare i32 @foo ^ :14:1: note: scanning

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

This revision was automatically updated to reflect the committed changes. Closed by commit rL317605: New clang option -fno-plt which avoids the PLT and lazy binding while making… (authored by tmsriram). Changed prior to commit: https://reviews.llvm.org/D39079?vs=119950=121947#toc Repository:

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

tmsriram added a comment. In https://reviews.llvm.org/D39079#905454, @joerg wrote: > In https://reviews.llvm.org/D39079#905396, @rnk wrote: > > > In https://reviews.llvm.org/D39079#905372, @joerg wrote: > > > > > Why again is this a good idea? > > > > > > It saves the direct jump to the PLT,

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

tmsriram added a comment. In https://reviews.llvm.org/D39079#905468, @rnk wrote: > In https://reviews.llvm.org/D39079#905454, @joerg wrote: > > > It also increases the pressure on the branch predictor, so it is not really > > black and white. > > > I don't understand this objection. I'm

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

tmsriram added a comment. In https://reviews.llvm.org/D39079#905423, @rnk wrote: > In https://reviews.llvm.org/D39079#905395, @joerg wrote: > > > Let me phrase it differently. What is this patch (and the matching backend > > PR) supposed to achieve? There are effectively two ways to get rid of

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

tmsriram added a comment. In https://reviews.llvm.org/D39079#905353, @hfinkel wrote: > Noting that, as @vit9696 pointed out in https://reviews.llvm.org/D38554, > this does not suppress uses of the PLT that occur from > backend/optimizer-generated functions (e.g., calls into compiler-rt and >

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

tmsriram added a comment. In https://reviews.llvm.org/D39079#905372, @joerg wrote: > Why again is this a good idea? This is an even worse hack than -Bsymbolic, > the latter at least is visible in ELF header without code inspection. This is > breaking core premises of ELF. Could you elaborate

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

tmsriram added a comment. In https://reviews.llvm.org/D39079#905519, @joerg wrote: > In https://reviews.llvm.org/D39079#905468, @rnk wrote: > > > In https://reviews.llvm.org/D39079#905454, @joerg wrote: > > > > > It also increases the pressure on the branch predictor, so it is not > > > really

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

tmsriram added a comment. Ping. How do we take this forward? https://reviews.llvm.org/D39079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42217: Set Module Metadata "AvoidPLT" when -fno-plt is used.

tmsriram created this revision. tmsriram added reviewers: rnk, rafael. Set Module Metadata "AvoidPLT" when -fno-plt is used. Please see: https://reviews.llvm.org/D42216 for more details. This allows processing of calls to intrinsics easily. https://reviews.llvm.org/D42217 Files:

[PATCH] D42217: Set Module Metadata "AvoidPLT" when -fno-plt is used.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL325961: Set Module Metadata RtLibUseGOT when fno-plt is used. (authored by tmsriram, committed by ). Herald added a

[PATCH] D42217: Set Module Metadata "AvoidPLT" when -fno-plt is used.

tmsriram added a comment. Ping. https://reviews.llvm.org/D42217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42217: Set Module Metadata "AvoidPLT" when -fno-plt is used.

tmsriram added a comment. Ping. This patch goes with https://reviews.llvm.org/D42216, Rafael can you approve this too if you think it is ok? Thanks. https://reviews.llvm.org/D42217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D42217: Set Module Metadata "AvoidPLT" when -fno-plt is used.

tmsriram updated this revision to Diff 135144. tmsriram added a comment. Updated patch. https://reviews.llvm.org/D42217 Files: lib/CodeGen/CodeGenModule.cpp test/CodeGen/noplt.c Index: test/CodeGen/noplt.c === ---

[PATCH] D42217: Set Module Metadata "AvoidPLT" when -fno-plt is used.

tmsriram updated this revision to Diff 130500. tmsriram added a comment. METADATA tag changed to "RtLibUseGOT". Also see: https://reviews.llvm.org/D42224/ https://reviews.llvm.org/D42217 Files: lib/CodeGen/CodeGenModule.cpp test/CodeGen/noplt.c Index: test/CodeGen/noplt.c

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

tmsriram marked 13 inline comments as done. tmsriram added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:341 +CODEGENOPT(RelocateWithSymbols, 1, 0) + /// Whether we should use the undefined behaviour optimization for control flow

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

tmsriram updated this revision to Diff 222035. tmsriram added a comment. Updated patch to address the comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Files: clang/include/clang/Basic/CodeGenOptions.def

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

tmsriram created this revision. tmsriram added a reviewer: rnk. Herald added a project: clang. Options for basic block sections, unique internal linkage function names. This is part of the Propeller framework to do post link code layout optimizations. Please see the RFC here:

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

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

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

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

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((), true); +if

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

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( +

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

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

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

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

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:

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

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,

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

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

tmsriram added a comment. In D68049#1870094 , @tmsriram wrote: > In D68049#1868623 , @MaskRay wrote: > > > > In D68049#1865967 , @MaskRay > > > wrote: > > > If you don't

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

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

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

tmsriram added a comment. In D68049#1870094 , @tmsriram wrote: > In D68049#1868623 , @MaskRay wrote: > > > > In D68049#1865967 , @MaskRay > > > wrote: > > > If you don't

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

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

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

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

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

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

tmsriram updated this revision to Diff 238552. tmsriram added a comment. Updated to top of trunk, bug fixes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Files: clang/include/clang/Basic/CodeGenOptions.def

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

tmsriram updated this revision to Diff 238620. tmsriram added a comment. clang-formatted. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Basic/CodeGenOptions.h

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

tmsriram added a comment. In D68049#1972297 , @dblaikie wrote: > In D68049#1971276 , @MaskRay wrote: > > > In D68049#1970825 , @tmsriram > > wrote: > > > > > Ping. > > > >

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

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

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:

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

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

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] D73307: Unique Names for Functions with Internal Linkage

tmsriram added a comment. In D73307#1989924 , @erichkeane wrote: > In D73307#1989740 , @rnk wrote: > > > In D73307#1978140 , @tmsriram > > wrote: > > > > > In

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

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

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:

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

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

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

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 >

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

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:

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

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:

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

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?

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

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

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] D73307: Unique Names for Functions with Internal Linkage

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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/

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

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

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

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:

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

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

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

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

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

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

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: >

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

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.

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

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 -

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

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

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

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

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

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

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

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

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

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

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] D87921: Fix -funique-internal-linkage-names to work with -O2 and new pass manager

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

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

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

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

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:

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

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] D89617: Prepend "uniq" to symbol names hash with -funique-internal-linkage-names

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

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

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

tmsriram added a comment. In D93747#2482726 , @hoy wrote: > In D93747#2482519 , @tmsriram wrote: > >> In D93747#2481494 , @dblaikie wrote: >> >>> In D93747#2481383

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

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

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

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

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

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

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

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

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<["-"],

  1   2   >