Page MenuHomePhabricator

snehasish (Snehasish Kumar)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 10 2017, 12:15 PM (210 w, 2 d)

Recent Activity

Mon, May 24

snehasish added a comment to D102900: [llvm-readobj] Print function names with `--bb-addr-map`..

Agree with tmsriram@ the quadratic behaviour needs to be addressed, though I think we should split it out as a separate change which can be committed first. That would make it easier for folks to review independently. Thanks!

Mon, May 24, 9:39 AM · Restricted Project

May 13 2021

snehasish accepted D102246: [SampleFDO] New hierarchical discriminator for Flow Sensitive SampleFDO.

lgtm, however please wait a bit for any follow up comments from other reviewers. Thanks!

May 13 2021, 2:32 PM · Restricted Project
snehasish added inline comments to D102246: [SampleFDO] New hierarchical discriminator for Flow Sensitive SampleFDO.
May 13 2021, 12:50 PM · Restricted Project

Apr 28 2021

snehasish accepted D101311: Basic block sections for functions with implicit-section-name attribute.

lgtm

Apr 28 2021, 12:03 PM · Restricted Project

Apr 23 2021

snehasish committed rG3da0aeea080f: [NFC] Use hasSection instead of getSection().empty() (authored by snehasish).
[NFC] Use hasSection instead of getSection().empty()
Apr 23 2021, 10:01 AM

Apr 21 2021

snehasish committed rG8077d0ff5c66: [CodeGen] Do not split functions with attr "implicit-section-name". (authored by snehasish).
[CodeGen] Do not split functions with attr "implicit-section-name".
Apr 21 2021, 9:54 PM
snehasish closed D101004: [CodeGen] Do not split functions with attr "implicit-section-name"..
Apr 21 2021, 9:54 PM · Restricted Project
snehasish added inline comments to D101004: [CodeGen] Do not split functions with attr "implicit-section-name"..
Apr 21 2021, 9:46 PM · Restricted Project
snehasish updated the diff for D101004: [CodeGen] Do not split functions with attr "implicit-section-name"..

Revert hasSection() since it is not semantically equivalent to getSection().empty().

Apr 21 2021, 8:56 PM · Restricted Project
snehasish updated the diff for D101004: [CodeGen] Do not split functions with attr "implicit-section-name"..

Use hasSection() in the condition.

Apr 21 2021, 5:27 PM · Restricted Project
snehasish requested review of D101004: [CodeGen] Do not split functions with attr "implicit-section-name"..
Apr 21 2021, 4:42 PM · Restricted Project

Mar 30 2021

snehasish added inline comments to D99123: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO).
Mar 30 2021, 3:15 PM · Restricted Project

Mar 29 2021

snehasish added inline comments to D99123: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO).
Mar 29 2021, 11:10 AM · Restricted Project
snehasish added inline comments to D99123: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO).
Mar 29 2021, 9:45 AM · Restricted Project

Mar 22 2021

snehasish added a comment to D95209: [CodeGen] Port Machine Function Splitter from ELF to COFF.

Yes, definitely adding it in a separate patch is a good idea :) Lets submit that patch first, before we submit with this one (otherwise any large binary generated by this patch is probably going to be broken).
It should include the necessary code changes for windows unwind info. I think the code modifications for that change should be in files: llvm/llvm-project/llvm/lib/MC/MCWin64EH.cpp and llvm/llvm-project/llvm/lib/CodeGen/AsmPrinter/WinException.cpp (this one will have changes for C++ programs with exceptions enabled).

Mar 22 2021, 10:48 AM · Restricted Project

Mar 19 2021

snehasish added a comment to D95209: [CodeGen] Port Machine Function Splitter from ELF to COFF.

Hi Tao, since it looks like you are making progress on this! I would suggest adding the Windows exception handling test for basic block sections instead of MFS in particular. It will simplify the test since you don't need profile information and we will get better confidence for the Propeller framework overall. It would be equivalent to this test: llvm/test/CodeGen/X86/basic-block-sections-eh.ll

Mar 19 2021, 10:45 AM · Restricted Project

Feb 17 2021

snehasish added a comment to D96393: [CodeGen] Add Machine Function Splitter and DebugInfo composite testcase.

As for Windows COFF, is it also not MFS but MFS dependent basic block sections may be incorrect for Windows native debuggers, so need to extend basic block sections debug info test at first?

Feb 17 2021, 10:36 PM · Restricted Project
snehasish added a comment to D96393: [CodeGen] Add Machine Function Splitter and DebugInfo composite testcase.

Thanks for reminding the discussion of not adding a separate debug info test for function splitting! I’ll merge the test into test/DebugInfo/X86/basic-block-sections_1.ll.

Please do not merge this test. Apologies if I was unclear earlier - there is no need for this test since the functionality is already tested in test/DebugInfo/X86/basic-block-sections_1.ll
The machine function splitter (MFS) pass uses basic block sections so if the dwarf debug info generated by basic block sections is correct then MFS is also correct.

Feb 17 2021, 8:33 PM · Restricted Project

Feb 11 2021

snehasish committed rG2c7077e67dc7: [CodeGen] Split out cold exception handling pads. (authored by snehasish).
[CodeGen] Split out cold exception handling pads.
Feb 11 2021, 11:25 AM
snehasish closed D96372: [CodeGen] Split out cold exception handling pads..
Feb 11 2021, 11:25 AM · Restricted Project
snehasish committed rGd079dbc59189: [CodeGen] Basic block sections should take precendence over splitting. (authored by snehasish).
[CodeGen] Basic block sections should take precendence over splitting.
Feb 11 2021, 11:15 AM
snehasish closed D96392: [CodeGen] Basic block sections should take precedence over splitting..
Feb 11 2021, 11:15 AM · Restricted Project

Feb 10 2021

snehasish added a comment to D96372: [CodeGen] Split out cold exception handling pads..

Thanks for the review.

Feb 10 2021, 6:27 PM · Restricted Project
snehasish updated the diff for D96372: [CodeGen] Split out cold exception handling pads..

Spell out types instead of auto.

Feb 10 2021, 6:25 PM · Restricted Project
snehasish added a comment to D96393: [CodeGen] Add Machine Function Splitter and DebugInfo composite testcase.

We have discussed this approach and decided against adding a separate debug info test for function splitting (see the discussion in D90717). The underlying functionality is tested in test/DebugInfo/X86/basic-block-sections_1.ll. Based on the direction in your previous patch (D95209) - emitting dwarf based debug info for PE/COFF executables has limited utility as far as I can tell. While tooling under cygwin will be able to use this information most Windows native debuggers may not support reading from dwarf (eg. libdwarf started supporting PE mid-2019: https://www.prevanders.net/dwarf.html#libdwarf-20190104). For Windows, we need to look into the CodeView format (see the slides from https://llvm.org/devmtg/2016-11/Slides/Kleckner-CodeViewInLLVM.pdf).

Feb 10 2021, 1:53 PM · Restricted Project
snehasish added a comment to D96372: [CodeGen] Split out cold exception handling pads..

I tried this patch on an instrumented build of mysql server. With this patch, an additional 28KB of text is split out from hot functions, a small but non-trivial improvement considering the size of the .text.hot section is ~1MB.

Feb 10 2021, 11:05 AM · Restricted Project

Feb 9 2021

snehasish added a comment to D96372: [CodeGen] Split out cold exception handling pads..

Yes, this would cause an issue if we had hot and cold landing pads. I've updated the logic to account for this and added a test. PTAL, thanks!

Feb 9 2021, 9:41 PM · Restricted Project
snehasish updated the diff for D96372: [CodeGen] Split out cold exception handling pads..

Extend the check to account of multiple landing pads with difference in hotness.
Add a new test to verify the expected behaviour.

Feb 9 2021, 9:39 PM · Restricted Project
snehasish retitled D96392: [CodeGen] Basic block sections should take precedence over splitting. from [CodeGen] Basic block sections should take precendence over splitting. to [CodeGen] Basic block sections should take precedence over splitting..
Feb 9 2021, 8:45 PM · Restricted Project
snehasish added a comment to D96393: [CodeGen] Add Machine Function Splitter and DebugInfo composite testcase.

Can you elaborate on the motivation behind this test?

Feb 9 2021, 8:33 PM · Restricted Project
snehasish updated the diff for D96392: [CodeGen] Basic block sections should take precedence over splitting..

Added FIXME to clarify that Labels and splitting can be enabled together but we don't support it yet.

Feb 9 2021, 8:07 PM · Restricted Project
snehasish requested review of D96392: [CodeGen] Basic block sections should take precedence over splitting..
Feb 9 2021, 7:28 PM · Restricted Project
snehasish requested review of D96372: [CodeGen] Split out cold exception handling pads..
Feb 9 2021, 2:25 PM · Restricted Project

Jan 28 2021

snehasish accepted D95593: Detect Source Drift with Propeller's basic-block-sections=list=.

lgtm

Jan 28 2021, 6:57 PM · Restricted Project
snehasish added inline comments to D95593: Detect Source Drift with Propeller's basic-block-sections=list=.
Jan 28 2021, 10:10 AM · Restricted Project

Jan 26 2021

snehasish accepted D95495: Emit metadata if there is a profile hash mismatch.

lgtm

Jan 26 2021, 9:56 PM · Restricted Project
snehasish added inline comments to D95495: Emit metadata if there is a profile hash mismatch.
Jan 26 2021, 9:22 PM · Restricted Project
snehasish requested changes to D95209: [CodeGen] Port Machine Function Splitter from ELF to COFF.

IMHO we should not ship the split functions feature targeting COFF without more testing. The machine function splitter relies on basic block sections and we should ensure that the debug information/call frame information generated by basic block sections is appropriate for COFF based consumers. Perhaps we can start by extending the existing CFI and DebugInfo tests in llvm/test/CodeGen/X86/cfi-basic-block-sections-1.ll and llvm/test/DebugInfo/X86/basic-block-sections_1.ll (in a separate patch).

Jan 26 2021, 10:42 AM · Restricted Project

Dec 8 2020

snehasish added a comment to D92882: [MBP] Prevent rotating a chain contains entry block.

Can we add a test for this?

Dec 8 2020, 2:35 PM · Restricted Project

Nov 9 2020

snehasish committed rG67cbd20469f1: [llvm] Check the debug info line table for basic block sections. (authored by snehasish).
[llvm] Check the debug info line table for basic block sections.
Nov 9 2020, 12:21 PM
snehasish closed D90989: [llvm] Check the debug info line table for basic block sections..
Nov 9 2020, 12:21 PM · Restricted Project
snehasish added a comment to D90989: [llvm] Check the debug info line table for basic block sections..

Thanks for the review David. I've added the addresses to the test for now. Extending the verbose output for dwarfdump seems like a good idea, we will look into adding that in the future.

Nov 9 2020, 11:24 AM · Restricted Project
snehasish updated the diff for D90989: [llvm] Check the debug info line table for basic block sections..

Check addresses as well, simplify triple.

Nov 9 2020, 11:11 AM · Restricted Project

Nov 6 2020

snehasish abandoned D90717: [llvm] Add a test for debug info generated with split functions..
Nov 6 2020, 4:31 PM · Restricted Project
snehasish requested review of D90989: [llvm] Check the debug info line table for basic block sections..
Nov 6 2020, 4:26 PM · Restricted Project

Nov 5 2020

snehasish added inline comments to D90717: [llvm] Add a test for debug info generated with split functions..
Nov 5 2020, 6:15 PM · Restricted Project
snehasish added a comment to D90717: [llvm] Add a test for debug info generated with split functions..

Thanks for the review all. I've updated the test to use llvm-dwarfdump to check that the line table is as expected for the hot and cold parts.

Nov 5 2020, 3:39 PM · Restricted Project
snehasish updated the diff for D90717: [llvm] Add a test for debug info generated with split functions..

Updates based on review comments.

Nov 5 2020, 3:32 PM · Restricted Project

Nov 3 2020

snehasish added a comment to D90717: [llvm] Add a test for debug info generated with split functions..

Thanks for the quick review @saugustine , I'll wait for @tmsriram take a look.

Nov 3 2020, 3:37 PM · Restricted Project
snehasish updated the diff for D90717: [llvm] Add a test for debug info generated with split functions..

Add UNSUPPORTED tag for windows and mac os.

Nov 3 2020, 3:35 PM · Restricted Project
snehasish requested review of D90717: [llvm] Add a test for debug info generated with split functions..
Nov 3 2020, 2:29 PM · Restricted Project

Oct 26 2020

snehasish accepted D90081: Test lldb backtraces with machine function splitter.

lgtm, thanks for adding the test.

Oct 26 2020, 10:25 AM · Restricted Project

Oct 24 2020

snehasish added inline comments to D90081: Test lldb backtraces with machine function splitter.
Oct 24 2020, 10:44 AM · Restricted Project

Oct 23 2020

snehasish added inline comments to D90081: Test lldb backtraces with machine function splitter.
Oct 23 2020, 7:00 PM · Restricted Project

Oct 15 2020

snehasish added inline comments to D89423: Explicitly check for entry basic block, rather than relying on MachineBasicBlock::pred_empty..
Oct 15 2020, 10:27 AM · Restricted Project

Oct 14 2020

snehasish committed rG24bf6ff4e08f: [llvm] Update default cutoff threshold for machine function splitter. (authored by snehasish).
[llvm] Update default cutoff threshold for machine function splitter.
Oct 14 2020, 12:51 PM
snehasish closed D89085: [llvm] Update default cutoff threshold for machine function splitter..
Oct 14 2020, 12:51 PM · Restricted Project
snehasish committed rG77638a5343d5: [llvm] Set the default for -bbsections-cold-text-prefix to .text.split. (authored by snehasish).
[llvm] Set the default for -bbsections-cold-text-prefix to .text.split.
Oct 14 2020, 12:29 PM
snehasish closed D88997: Set the default for -bbsections-cold-text-prefix to .text.split..
Oct 14 2020, 12:29 PM · Restricted Project

Oct 13 2020

snehasish updated the diff for D88997: Set the default for -bbsections-cold-text-prefix to .text.split..

Rebase.

Oct 13 2020, 11:13 PM · Restricted Project
snehasish added a comment to D89085: [llvm] Update default cutoff threshold for machine function splitter..

Thanks for the review.

Oct 13 2020, 11:10 PM · Restricted Project
snehasish updated the diff for D89085: [llvm] Update default cutoff threshold for machine function splitter..

Add comments, rebase.

Oct 13 2020, 11:09 PM · Restricted Project

Oct 8 2020

snehasish added a comment to D88997: Set the default for -bbsections-cold-text-prefix to .text.split..

Thanks for the review.

Oct 8 2020, 6:38 PM · Restricted Project
snehasish updated the diff for D88997: Set the default for -bbsections-cold-text-prefix to .text.split..

Update prefix to .text.split.

Oct 8 2020, 6:36 PM · Restricted Project
snehasish added a comment to D89085: [llvm] Update default cutoff threshold for machine function splitter..

PTAL, thanks!

Oct 8 2020, 6:32 PM · Restricted Project
snehasish updated the diff for D89085: [llvm] Update default cutoff threshold for machine function splitter..

Spell out TTI as TargetTransformInfo, rebase.

Oct 8 2020, 6:32 PM · Restricted Project
snehasish updated the diff for D88997: Set the default for -bbsections-cold-text-prefix to .text.split..

Update comment and rebase.

Oct 8 2020, 6:24 PM · Restricted Project
snehasish added a comment to D89085: [llvm] Update default cutoff threshold for machine function splitter..

This pass is only enabled on X86 platforms (D87047 for clang options and platform check). Longer term it does make sense to move it to TTI so I've added a FIXME to get this change off the critical path and I'll follow up with a refactoring change.

Oct 8 2020, 6:17 PM · Restricted Project
snehasish updated the diff for D89085: [llvm] Update default cutoff threshold for machine function splitter..

Add a FIXME to move defaults to TTI.

Oct 8 2020, 6:14 PM · Restricted Project
snehasish requested review of D89085: [llvm] Update default cutoff threshold for machine function splitter..
Oct 8 2020, 5:47 PM · Restricted Project

Oct 7 2020

snehasish updated the diff for D88997: Set the default for -bbsections-cold-text-prefix to .text.split..

Updated tests to use .text.split. prefix.

Oct 7 2020, 12:10 PM · Restricted Project
snehasish abandoned D88996: Set the default for -bbsections-cold-text-prefix to .text.split..
Oct 7 2020, 11:56 AM · Restricted Project
snehasish updated the summary of D88996: Set the default for -bbsections-cold-text-prefix to .text.split..
Oct 7 2020, 11:56 AM · Restricted Project
snehasish requested review of D88997: Set the default for -bbsections-cold-text-prefix to .text.split..
Oct 7 2020, 11:54 AM · Restricted Project
snehasish requested review of D88996: Set the default for -bbsections-cold-text-prefix to .text.split..
Oct 7 2020, 11:50 AM · Restricted Project

Oct 6 2020

snehasish added inline comments to D88517: [BasicBlockSections] Make sure that the labels for address-taken blocks are emitted after switching the seciton..
Oct 6 2020, 5:00 PM · Restricted Project
snehasish accepted D88517: [BasicBlockSections] Make sure that the labels for address-taken blocks are emitted after switching the seciton..

lgtm with some minor comments.

Oct 6 2020, 3:29 PM · Restricted Project

Sep 25 2020

snehasish removed reviewers for D88041: [lld] Add a flag to enable split machine functions for LTO.: MaskRay, tejohnson, espindola.
Sep 25 2020, 1:15 PM · Restricted Project
snehasish added a comment to D88041: [lld] Add a flag to enable split machine functions for LTO..

Let me look into module flags a little bit and I'll come back to this patch once I have a better understanding. Thanks for the comments!

Sep 25 2020, 1:14 PM · Restricted Project

Sep 24 2020

snehasish committed rGd2696dec45cd: [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different… (authored by snehasish).
[llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different…
Sep 24 2020, 3:30 PM
snehasish closed D87813: [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different section..
Sep 24 2020, 3:30 PM · Restricted Project
snehasish added a comment to D87813: [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different section..

Thanks for the review.

Sep 24 2020, 3:16 PM · Restricted Project
snehasish updated the diff for D87813: [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different section..

Update comment, test.

Sep 24 2020, 3:13 PM · Restricted Project
snehasish committed rG070555c6c008: [lld] Make -z keep-text-section-prefix recognize .text.split. as a prefix. (authored by snehasish).
[lld] Make -z keep-text-section-prefix recognize .text.split. as a prefix.
Sep 24 2020, 3:06 PM
snehasish closed D87840: [lld] Make -z keep-text-section-prefix recognize .text.split. as a prefix..
Sep 24 2020, 3:06 PM · Restricted Project
snehasish added a comment to D87840: [lld] Make -z keep-text-section-prefix recognize .text.split. as a prefix..

Updated description and tests, PTAL thanks!

Sep 24 2020, 2:45 PM · Restricted Project
snehasish retitled D87840: [lld] Make -z keep-text-section-prefix recognize .text.split. as a prefix. from [lld] Add a new known text prefix - ".text.split." to [lld] Make -z keep-text-section-prefix recognize .text.split. as a prefix..
Sep 24 2020, 2:44 PM · Restricted Project
snehasish updated the diff for D87840: [lld] Make -z keep-text-section-prefix recognize .text.split. as a prefix..

Update description, add test.

Sep 24 2020, 2:44 PM · Restricted Project
snehasish retitled D87813: [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different section. from [llvm]Add an option to emit cold clusters to a different section. to [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different section..
Sep 24 2020, 2:25 PM · Restricted Project
snehasish updated the diff for D87813: [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different section..

Update git commit message to specify option.

Sep 24 2020, 2:25 PM · Restricted Project
snehasish added a comment to D87813: [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different section..

PTAL, thanks!

Sep 24 2020, 2:20 PM · Restricted Project
snehasish updated the diff for D87813: [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different section..

Add another check for the test.

Sep 24 2020, 2:19 PM · Restricted Project
snehasish updated the diff for D87813: [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different section..

Document flag, tighten test, rename var and option for clarity.

Sep 24 2020, 2:17 PM · Restricted Project
snehasish added a comment to D87787: Temporary fix for debug loc list bug with basic block sections.

ping @tmsriram
Lets push this if it's good to go?

Sep 24 2020, 8:42 AM · Restricted Project

Sep 22 2020

snehasish updated subscribers of D88041: [lld] Add a flag to enable split machine functions for LTO..

Is it expected that the user will want to specify this only at link time and not during compile time? If it is normally specified in the compile step, you should consider adding it to the IR in some fashion (e.g. function attributes). The advantage is that the user doesn't need to pass different options for the LTO and non-LTO cases.

Ideally we should only have to specify this once. However, using function attributes doesn't seem ideal since the pass will be scheduled and repeatedly invoked only to return without actually running the pass. It would be cleaner to marshal the codegen specific options from the compile invocation and restore them for the LTO step. There are a couple of other codegen options which would also benefit from this approach --lto-unique-basic-block-section-names, --lto-basic-block-sections=<value>. @mtrofin pointed out that -fembed-bitcode saves the invocation in .llvmcmd. A similar approach to stash codegen specific options always for LTO to pick up and enable might be less intrusive. However, this is a larger effort and for current LTO builds it would be nice to have a command line option to enable it. WDYT about this alternative?

Sep 22 2020, 3:42 PM · Restricted Project

Sep 21 2020

snehasish added a comment to D87813: [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different section..

ping @tmsriram @MaskRay

Sep 21 2020, 2:42 PM · Restricted Project
snehasish retitled D87813: [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different section. from [llvm][lld] Add an option to emit cold clusters to a different section. to [llvm]Add an option to emit cold clusters to a different section..
Sep 21 2020, 2:42 PM · Restricted Project
snehasish updated the diff for D87813: [llvm] Add -bbsections-cold-text-prefix to emit cold clusters to a different section..

Rebase and update git commit message.

Sep 21 2020, 2:41 PM · Restricted Project
snehasish requested review of D88041: [lld] Add a flag to enable split machine functions for LTO..
Sep 21 2020, 12:48 PM · Restricted Project

Sep 18 2020

snehasish committed rGb86f1af42395: [clang] Remove profile available check for fsplit-machine-functions. (authored by snehasish).
[clang] Remove profile available check for fsplit-machine-functions.
Sep 18 2020, 3:20 PM