Page MenuHomePhabricator

vsk (Vedant Kumar)
User

Projects

User Details

User Since
Jul 8 2015, 10:26 AM (210 w, 1 d)

Recent Activity

Tue, Jul 16

vsk added a comment to D64645: DAG: Handle dbg_value for arguments split into multiple subregs.

I think this looks reasonable, but am not familiar enough with the code to approve.

Tue, Jul 16, 1:43 PM

Mon, Jul 15

vsk added a comment to D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO".

Looks reasonable to me -- mind waiting for another +1 on this to be safe? @eugenis, any thoughts?

Mon, Jul 15, 12:32 PM · Restricted Project, Restricted Project, Restricted Project

Fri, Jul 12

vsk accepted D64500: [DebugInfo] Remove call sites when eliminating unreachable blocks.

I just want to add that I was a bit hesitant about generalizing the removal as I think it can be quite hard to tell when call site information has been removed, so I thought it would be better to have asserts trigger for each individual case, so that we can detect and assess what to do there, rather than dropping the information silently at the risk of false negatives.

Fri, Jul 12, 11:25 AM · Restricted Project, debug-info
vsk added a reviewer for D64645: DAG: Handle dbg_value for arguments split into multiple subregs: debug-info.
Fri, Jul 12, 10:59 AM

Thu, Jul 11

vsk added a reviewer for D64595: [Debuginfo][SROA] Need to handle dbg.value in SROA pass.: debug-info.

Thanks for the patch!

Thu, Jul 11, 4:07 PM · Restricted Project, debug-info
vsk committed rG8bd521472643: Revert "[CGDebugInfo] Simplify EmitFunctionDecl parameters, NFC" (authored by vsk).
Revert "[CGDebugInfo] Simplify EmitFunctionDecl parameters, NFC"
Thu, Jul 11, 12:31 PM
vsk planned changes to D64540: [CGDebugInfo] Simplfiy EmitFunctionDecl parameters, NFC.
Thu, Jul 11, 12:31 PM · Restricted Project
vsk reopened D64540: [CGDebugInfo] Simplfiy EmitFunctionDecl parameters, NFC.

Sorry about that, reopening per Eric's comment.

Thu, Jul 11, 12:31 PM · Restricted Project
vsk committed rGbdf8198d4cbd: [CGDebugInfo] Simplify EmitFunctionDecl parameters, NFC (authored by vsk).
[CGDebugInfo] Simplify EmitFunctionDecl parameters, NFC
Thu, Jul 11, 12:13 PM
vsk accepted D64498: [profile][test] Fix Profile-* :: instrprof-merge.c etc. on SPARC.

Thanks, lgtm.

Thu, Jul 11, 11:03 AM · Restricted Project, Restricted Project
vsk added a comment to D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO".

This looks reasonable to me. I'd prefer to disallow -fsanitize=function in combination with -fsanitize-minimal-runtime, as I'm not sure it's appropriate for the minimal runtime to handle SANITIZER_NON_UNIQUE_TYPEINFO incorrectly. If this has already been discussed elsewhere I'd appreciate a pointer.

Thu, Jul 11, 10:57 AM · Restricted Project, Restricted Project, Restricted Project
vsk added a comment to D64500: [DebugInfo] Remove call sites when eliminating unreachable blocks.

@dstenb The patch looks good! LGTM! Thanks for working on this!

Wdyt of addressing this problem in a more general way, e.g. in MachineInstruction::eraseFromParent itself?
edit: I meant to write MachineBasicBlock::eraseFromParent, but I think the point still stands.

That would be great and we would not need to worry about validity of the CallSitesInfo. But the thing is that we have a problem with the MI range deletion from the MBB (see for example MachineOutliner::outline or version before this feature for TargetInstrInfo::ReplaceTailWithBranchTo). Such deletions cannot be generalized. For now, there are just two of such cases but in further we may stumble across more.
The whole idea behind this assertion and update is to evade generalization in the destructor of the MachineInst (which is the safest but requires certain overhead) and to use the update only when necessary. Similarly will be with the eraseFromParent, it will require certain overhead.
Please let us know if you have an idea how to overcome those constraints.

Thu, Jul 11, 10:31 AM · Restricted Project, debug-info

Wed, Jul 10

vsk committed rG31c4d2a40d12: [CGDebugInfo] Fix -femit-debug-entry-values crash on os_log_helpers (authored by vsk).
[CGDebugInfo] Fix -femit-debug-entry-values crash on os_log_helpers
Wed, Jul 10, 5:11 PM
vsk added a comment to D64500: [DebugInfo] Remove call sites when eliminating unreachable blocks.

Wdyt of addressing this problem in a more general way, e.g. in MachineInstruction::eraseFromParent itself?

Wed, Jul 10, 5:03 PM · Restricted Project, debug-info
vsk created D64540: [CGDebugInfo] Simplfiy EmitFunctionDecl parameters, NFC.
Wed, Jul 10, 3:44 PM · Restricted Project
vsk requested changes to D63689: llvm-cov don't fail the entire invocation if one of the provided object doesn't contain coverage info.

(Marking this to reflect my comment from 5/24 to clear my review queue)

Wed, Jul 10, 2:42 PM · Restricted Project
vsk accepted D58450: [DebugInfo][MachineCSE] Don't try to copy-propagate debuginfo for every COPY seen.

Lgtm, thanks.

Wed, Jul 10, 2:37 PM · Restricted Project
vsk requested changes to D44672: [CodeGen] Disable UBSan for coroutine functions.

(Marking this to reflect my comment from 3/20/18 to clear my review queue)

Wed, Jul 10, 2:33 PM · Restricted Project
vsk resigned from D53578: [CodeGen] Fix clang test for gcov profiling (follow-up of D51974).

(Clearing my review queue - I believe the parent revision D51974 has been accepted, perhaps @marco-c can take a look)

Wed, Jul 10, 2:30 PM · Restricted Project
vsk resigned from D53732: [compiler-rt] [llvm-cov] Test for "Don't remap existing paths".
Wed, Jul 10, 2:30 PM · Restricted Project, Restricted Project
vsk resigned from D53729: [llvm-cov] Don't remap existing paths.

(Clearing my review queue - still think adding an 'unsupported' line would make sense if this remains an issue)

Wed, Jul 10, 2:30 PM · Restricted Project
vsk added inline comments to D58238: [DebugInfo] MachineSink: Insert undef DBG_VALUEs when sinking instructions, try to forward copies.
Wed, Jul 10, 2:07 PM · Restricted Project
vsk added a comment to D58191: [DebugInfo] Make postra sinking of DBG_VALUEs safe in the presence of subregisters.

This test looks reasonable. Is D58238 the only MachineSink change you have queued up?

Wed, Jul 10, 1:16 PM · Restricted Project
vsk committed rG5eb6ba060a2f: [CodeExtractor] Fix sinking of allocas with multiple bitcast uses (PR42451) (authored by vsk).
[CodeExtractor] Fix sinking of allocas with multiple bitcast uses (PR42451)
Wed, Jul 10, 9:34 AM
vsk committed rGf65f302cc7af: [CodeExtractor] Simplify findAllocas, NFC (authored by vsk).
[CodeExtractor] Simplify findAllocas, NFC
Wed, Jul 10, 9:34 AM
vsk added a comment to D64498: [profile][test] Fix Profile-* :: instrprof-merge.c etc. on SPARC.

This might need to be '__declspec(align(x))' for msvc. See sanitizer_internal_defs.h. Imho ideally the profile library would just share that file, or just use llvm/{ADT,Support}.

Wed, Jul 10, 9:29 AM · Restricted Project, Restricted Project

Tue, Jul 9

vsk updated the diff for D64463: [CodeExtractor] Fix sinking of allocas with multiple bitcast uses (PR42451).
  • Rebase on an NFC refactoring: D64467
Tue, Jul 9, 7:43 PM · Restricted Project
vsk created D64467: [CodeExtractor] Simplify findAllocas, NFC.
Tue, Jul 9, 7:43 PM · Restricted Project
vsk created D64463: [CodeExtractor] Fix sinking of allocas with multiple bitcast uses (PR42451).
Tue, Jul 9, 6:21 PM · Restricted Project
vsk committed rGd6c15b661ab0: [Profile] Support raw/indexed profiles larger than 4GB (authored by vsk).
[Profile] Support raw/indexed profiles larger than 4GB
Tue, Jul 9, 3:03 PM
vsk created D64437: [IR] Consolidate fixed metadata kind definitions (NFC).
Tue, Jul 9, 11:58 AM · Restricted Project

Fri, Jun 28

vsk added a comment to D58453: [DebugInfo][CGP] Limit placeDbgValues movement of dbg.value intrinsics.

The high-level plan (limit the amount of movement done in placeDbgValues, check for any fallout, and (hopefully) eventually eliminate all of it) sgtm.

Fri, Jun 28, 1:34 PM · Restricted Project
vsk added a comment to D58450: [DebugInfo][MachineCSE] Don't try to copy-propagate debuginfo for every COPY seen.

Looks reasonable to me.

Fri, Jun 28, 1:23 PM · Restricted Project
vsk accepted D63429: [DebugInfo] Avoid adding too much indirection to pointer-valued variables.

Thanks!

Fri, Jun 28, 12:50 PM · Restricted Project

Thu, Jun 27

vsk added a comment to D63429: [DebugInfo] Avoid adding too much indirection to pointer-valued variables.

This looks reasonable to me. Mind adding test coverage for simple tag_offset/fragment expressions?

Thu, Jun 27, 3:05 PM · Restricted Project
vsk accepted D56265: [DebugInfo] MCP: collect and update DBG_VALUEs encountered in local block.

Lgtm, thanks for working on this. I haven't touched MCP myself, so could you wait for another +1? + @bogner in case he has time to take a look.

Thu, Jun 27, 11:16 AM

Mon, Jun 24

vsk accepted D59225: [profile] Support for GCDA profiling in Fuchsia.

Thanks!

Mon, Jun 24, 1:34 PM · Restricted Project, Restricted Project
vsk added a comment to D63689: llvm-cov don't fail the entire invocation if one of the provided object doesn't contain coverage info.

Thanks for the patch.

Mon, Jun 24, 10:55 AM · Restricted Project
vsk added a comment to D59225: [profile] Support for GCDA profiling in Fuchsia.

This looks ok to me. In the longer term, I think compiler-rt would benefit from borrowing/extending llvm's generic file interface to abstract over platform differences. It doesn't seem ideal to me that libprofile/xray etc. have lots of platform-specific file I/O code.

Mon, Jun 24, 10:45 AM · Restricted Project, Restricted Project

Fri, Jun 21

vsk edited reviewers for D63671: [llvm-profdata] Avoid keeping reference to every files, added: danielcdh, xur, dnovillo, davidxl; removed: vsk.

Adding reviewers who are familiar with SamplePGO.

Fri, Jun 21, 2:10 PM · Restricted Project

Thu, Jun 20

vsk accepted D41111: [profile] Solaris ld supports __start___llvm_prof_data etc. labels.

Thanks, lgtm.

Thu, Jun 20, 10:30 AM · Restricted Project

Jun 13 2019

vsk committed rG1e4882c8906f: [Coverage] Speculative fix for r363325 for an older compiler (authored by vsk).
[Coverage] Speculative fix for r363325 for an older compiler
Jun 13 2019, 5:01 PM
vsk committed rG901d04fc6df8: [Coverage] Load code coverage data from archives (authored by vsk).
[Coverage] Load code coverage data from archives
Jun 13 2019, 1:46 PM
vsk updated the diff for D63232: [Coverage] Load code coverage data from archives.
  • Add support for thin archives (and a test).
  • Update the llvm-cov CommandGuide doc.
Jun 13 2019, 12:12 PM · Restricted Project
vsk updated the diff for D63232: [Coverage] Load code coverage data from archives.
  • Address review feedback.
Jun 13 2019, 10:30 AM · Restricted Project
vsk added inline comments to D63232: [Coverage] Load code coverage data from archives.
Jun 13 2019, 10:30 AM · Restricted Project

Jun 12 2019

vsk created D63232: [Coverage] Load code coverage data from archives.
Jun 12 2019, 3:49 PM · Restricted Project

Jun 4 2019

vsk accepted D62623: Reduce memory consumption of coverage dumps.

LGTM.

Jun 4 2019, 3:32 PM · Restricted Project

Jun 3 2019

vsk added inline comments to D62623: Reduce memory consumption of coverage dumps.
Jun 3 2019, 11:25 AM · Restricted Project

May 8 2019

vsk accepted D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal..

Lgtm, thanks!

May 8 2019, 9:25 AM · Restricted Project

May 7 2019

vsk added inline comments to D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal..
May 7 2019, 5:18 PM · Restricted Project

May 2 2019

vsk added inline comments to D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal..
May 2 2019, 3:34 PM · Restricted Project
vsk added a comment to D61454: [CodeGen][ObjC] Remove the leading 'l_' from ObjC symbols and make private symbols in the __DATA segment internal..

Thanks for working on this!

May 2 2019, 11:33 AM · Restricted Project

Apr 26 2019

vsk requested changes to D34499: Expose __gcov_flush for parity with libgcov in the gcc project.
Apr 26 2019, 2:02 PM · Restricted Project, Restricted Project
vsk added a comment to D34499: Expose __gcov_flush for parity with libgcov in the gcc project.

Thanks for fixing the warning. To run the tests, try 'ninja check-profile' from your build directory. The failure from earlier just meant that the CHECK lines in a few *.gcov files needed to be updated. Here's a version of this patch which fixes up the tests which run on Darwin:

Apr 26 2019, 2:02 PM · Restricted Project, Restricted Project
vsk added a comment to D61198: [IRBuilder][DebugInfo] Don't pick DebugLocs for new instructions from debug intrinsics.

There's a tension here between finding a narrow solution to the invalid-location bug and having SetInsertPoint() do something principled with debug locations. See D39982 for a more detailed explanation of this. That previous attempt to change the IRBuilder API stalled because we couldn't find an automated way to fix up in-tree/out-of-tree uses (imho this is a hard problem).

Apr 26 2019, 11:06 AM · Restricted Project

Apr 25 2019

vsk added a comment to D60716: [DwarfDebug] Dump call site debug info into DWARF.

Thanks for working on this. I haven't gotten to 'collectCallSiteParameters' or the x86-specific bits yet, but have some initial feedback which I hope is useful --

Apr 25 2019, 4:15 PM · Restricted Project, debug-info
vsk added a comment to D58033: Add option for emitting dbg info for call site parameters.

I think a small extension to test/CodeGenCXX/dbg-info-all-calls-described.cpp for the 'Supported: DWARF4 + -femit_param_entry_values' case would be appropriate here.

Apr 25 2019, 2:18 PM · Restricted Project, debug-info
vsk added a comment to D60712: [DWARF] Add GNU extensions for call site info DWARF symbols.

Is there a separate patch which adds DWARF verifier support for these new attributes? If not, this could be the right vehicle for it.

Apr 25 2019, 2:16 PM · Restricted Project, debug-info
vsk accepted D61008: [NFC][Utils] deleteDeadLoop(): add an assert that exit block has some non-PHI instruction.

Thanks, lgtm.

Apr 25 2019, 2:15 PM · Restricted Project

Apr 19 2019

vsk committed rG282b26ec4d98: [GVN+LICM] Use line 0 locations for better crash attribution (authored by vsk).
[GVN+LICM] Use line 0 locations for better crash attribution
Apr 19 2019, 3:38 PM
vsk added a comment to D28390: [DWARF] LICM should null out the debug loc of hoisted loop invariant instructions.

See: https://reviews.llvm.org/D60913

Apr 19 2019, 12:06 PM · Restricted Project
vsk created D60913: [GVN+LICM] Use line 0 locations for better crash attribution.
Apr 19 2019, 12:06 PM · Restricted Project
vsk added a comment to D28390: [DWARF] LICM should null out the debug loc of hoisted loop invariant instructions.

@danielcdh @wolfgangp -- Would switching to line 0 locations for hoisted instructions have an adverse effect on Sample PGO?

It does indeed seem preferrable, since the hoisted instruction is currently attributed to the same line as the previous instruction, which is not what we want. I think our usage and understanding of 0 linenumbers has evolved since this change was made and we should examine all the places where we're removing the line number and see if setting it to 0 instead is better.

Apr 19 2019, 11:19 AM · Restricted Project

Apr 18 2019

vsk added a comment to D28390: [DWARF] LICM should null out the debug loc of hoisted loop invariant instructions.

ping

Apr 18 2019, 4:52 PM · Restricted Project
vsk added a comment to D60831: [DebugInfo@O2][LoopVectorize] pr39024: Vectorized code linenos step through loop even after completion.

As the lead of a project building profiling tools, I am strongly against having any instructions map to line 0.

This is probably not what you meant, but for completeness I feel like I should point out that there are many legitimate situations where LLVM generates a line 0 location. The most prominent example is instruction merging: Since both LLVM IR and DWARF currently require each PC address to map to exactly one source location, LLVM's will insert a line 0 location when it merges two instructions with distinct source locations. I can't speak for profiling, but at least on the debugger side, the consensus is that potentially misleading information is worse than no information, because if there is no way to distinguish "always correct" from "maybe correct" information, the user can't trust any information.

When merging happens, I don't see why mapping to 0 is better than attributing it to one of a set of locations that a fused operation came from. I see that as representative of reality.

I would prefer a rule stating that if two operations are merged, always pick the lexicographically least (file, line number) pair. In the case of mappings for merged instructions, I see either mapping as "correct".

Apr 18 2019, 4:46 PM · debug-info, Restricted Project

Apr 15 2019

vsk added a comment to D60586: [Clang] Conversion of a switch table to a bitmap is not profitable for -Os and -Oz compilation.

Why shouldn't llvm simply pick the right behavior for -Os/-Oz automatically?

Apr 15 2019, 3:28 PM · Restricted Project

Apr 12 2019

vsk accepted D59790: [DebugInfo][Docs] Document how variable location metadata is transformed through target codegen.

Looks great, thanks!

Apr 12 2019, 3:53 PM · Restricted Project
vsk added a comment to D60612: Cowardly refuse to insert instructions after a terminator..

Thanks for working on this. Two thoughts:

Apr 12 2019, 8:13 AM · Restricted Project

Apr 8 2019

vsk added a comment to D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting.

Ping.

Apr 8 2019, 9:40 AM · Restricted Project
vsk added a comment to D59715: [HotColdSplit] Reflect full cost of parameters in split penalty.

Ping.

Apr 8 2019, 9:37 AM · Restricted Project

Apr 4 2019

Herald added a project to D28390: [DWARF] LICM should null out the debug loc of hoisted loop invariant instructions: Restricted Project.

As an alternative to removing the debug location, what about setting a line 0 location (with appropriate scope/inlinedAt info) to the hoisted instruction? It seems like that allows debuggers to give more helpful/specific backtraces. It also doesn't affect stepping (at least not in lldb, which collapses line 0 ranges). Example:

Apr 4 2019, 9:40 PM · Restricted Project

Apr 3 2019

vsk updated subscribers of D60209: If conversion to conditional select for -Oz or -Os unprofitable for single operation blocks.

Thanks for working on this. I’d suggest splitting this into a sequence of three patches (with tests and accompanying code size numbers), so its effects can be understood better.

Apr 3 2019, 10:54 AM

Apr 2 2019

vsk added inline comments to D59790: [DebugInfo][Docs] Document how variable location metadata is transformed through target codegen.
Apr 2 2019, 7:20 PM · Restricted Project
vsk added a comment to D59790: [DebugInfo][Docs] Document how variable location metadata is transformed through target codegen.

Thanks so much for writing this down! Imho this is in pretty good shape (better than the drafts of similar docs I've thrown away). I've taken a look at the first third. I'll try to give a more detailed pass over the remaining bits later.

Apr 2 2019, 7:08 PM · Restricted Project
vsk committed rG37b0f9ad9531: [os_log] Mark os_log_helper `nounwind` (authored by vsk).
[os_log] Mark os_log_helper `nounwind`
Apr 2 2019, 10:43 AM
vsk committed rG9da8a68d6b54: [ArgPromotion] Set debug location at updated callsites (authored by vsk).
[ArgPromotion] Set debug location at updated callsites
Apr 2 2019, 10:42 AM
vsk committed rGc6bceec01a4b: [DebugInfo] Fix pr41180 : Loop Vectorization Debugify Failure (authored by vsk).
[DebugInfo] Fix pr41180 : Loop Vectorization Debugify Failure
Apr 2 2019, 10:28 AM
vsk added a comment to D34499: Expose __gcov_flush for parity with libgcov in the gcc project.

Sorry for the delay here. I am hitting two test failures with this patch applied on Darwin. Could you take a look?

Apr 2 2019, 10:20 AM · Restricted Project, Restricted Project

Apr 1 2019

vsk created D60113: [ArgPromotion] Set debug location at updated callsites.
Apr 1 2019, 10:42 PM · Restricted Project
vsk created D60108: [os_log] Mark os_log_helper `nounwind`.
Apr 1 2019, 6:43 PM · Restricted Project
vsk accepted D59944: [DebugInfo] Fix pr41180 : Loop Vectorization Debugify Failure.

Thanks, lgtm.

Apr 1 2019, 12:07 PM · Restricted Project

Mar 29 2019

vsk added a comment to D59715: [HotColdSplit] Reflect full cost of parameters in split penalty.

Ping.

Mar 29 2019, 11:45 AM · Restricted Project

Mar 25 2019

vsk updated the diff for D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting.

Use a function attribute to implement the -fsplit-cold-code option. This removes some complexity from the old/new PMs and ensures consistent behavior when LTO is enabled.

Mar 25 2019, 2:43 PM · Restricted Project

Mar 22 2019

vsk added a comment to D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting.
In D57265#1393453, @vsk wrote:

I'd prefer not adding this kind of state to PassBuilder. SplitColdCode is soemthing that refers to the construction of one particular pipeline, not to pipeline-building in general. It should be an argument passed down through the build*Pipeline calls.

I'm not sure I understand the distinction being made here -- could you elaborate? Isn't enabling a pass related to pipeline-building in general? If not, I don't see how arguments to build*Pipeline do become related to pipeline-building in general.

For context, I've modeled the addition of the SplitColdCode member to PassBuilder on PGOOpt (c.f. PGOOptions::RunProfileGen). One thing I like about this approach is that it doesn't require changing IR, or changing very many different APIs. But if it's really not reasonable, I'm happy to take another shot at it.

I cant say for PGOOpt in particular, but overall idea of PassBuilder is that, well, it is not "builder" as per "builder pattern".
You do not fill it with parts of a complex pipeline object and then press a magical "build" button.
Instead you ask it to build various "named" pipelines, or just parse it from text.
If you compare with PassManagerBuilder, which contains a dozen of various data members that seem to drive the pipeline construction,
PassBuilder contains only a few. And the purpose of PassBuilder members is to be used during individual *pass*es creation.

Say, you wont find OptLevel there. Instead it is being passed as an argument to build*Pipeline functions.

As for PGOOpts - it seems to be the only member that stands out.
And to me it looks like we should remove it from PassBuilder either, and start passing it to build*Pipeline functions as well.
But honestly, this is the first time I really looked through the PGOOpts code, so I might be well mistaken.

Mar 22 2019, 3:38 PM · Restricted Project
vsk updated the diff for D59715: [HotColdSplit] Reflect full cost of parameters in split penalty.
  • Increase test coverage.
Mar 22 2019, 2:24 PM · Restricted Project
vsk created D59715: [HotColdSplit] Reflect full cost of parameters in split penalty.
Mar 22 2019, 1:47 PM · Restricted Project

Mar 18 2019

vsk added a comment to D59417: [GVN] Add default debug location when constructing PHI nodes.
In D59417#1431369, @vsk wrote:

As for instructions at the start of the block - I'm pretty sure that a while back @probinson made a change to ensure that cascade locations couldn't occur at the start of a block (that we'd set to line zero down in the AsmPrinter/DwarfDebug if that occurred) so we shouldn't end up with super weird/layout-based cascade locations for instructions generated from phis at the start of blocks.

That makes sense. What if a block has multiple phis? I.e. if the first phi in a block has a location but the second phi doesn't, is that a bug, or do we expect the location to cascade?

Locations cascade, with one exception: If the first instruction after a label does not have an explicit source location, we emit line 0.

Mar 18 2019, 10:42 AM · Restricted Project

Mar 15 2019

vsk added a comment to D59417: [GVN] Add default debug location when constructing PHI nodes.
Phis often cannot assume a previous location (say, if they're at the start of a block).

Not sure I follow this bit - setting a zero location doesn't make them any easier to debug, does it? Or was it that the "cascade location" (whatever the previous instruction was) the source of confusion?

Mar 15 2019, 1:16 PM · Restricted Project
vsk added a comment to D59417: [GVN] Add default debug location when constructing PHI nodes.

I believe this is a patch to fix PR37964?

Hrm - if debugify diagnoses all instances of instructions without locations as buggy, that sounds very noisy to me. How does it handle all the other cases of merged locations that end up with no location? (if my recollection is accurate and the code is still doing this - merged locations that aren't the same and that aren't calls get no location, not a zero location)

Mar 15 2019, 11:31 AM · Restricted Project

Mar 14 2019

vsk accepted D59277: Speeding up llvm-cov export with multithreaded renderFiles implementation..

Thanks, lgtm.

Mar 14 2019, 9:12 AM · Restricted Project

Mar 13 2019

vsk added a comment to D59277: Speeding up llvm-cov export with multithreaded renderFiles implementation..

Thanks for working on this!

Mar 13 2019, 4:40 PM · Restricted Project
vsk added inline comments to D59277: Speeding up llvm-cov export with multithreaded renderFiles implementation..
Mar 13 2019, 10:40 AM · Restricted Project
vsk added a reviewer for D59288: [DebugInfoMetadata] Move main subprogram DIFlag into DISPFlags: steven_wu.

+ Steven, is debug info bitcode backwards compatibility important for the App Store? (I’m not 100% sure, but I think it is, in which case we’d need to 1) detect old DI metadata and 2) auto upgrade it?)

Mar 13 2019, 8:14 AM · Restricted Project

Mar 11 2019

vsk accepted D59206: [SimplifyCFG] Retain debug info when threading jumps with critical edges.

Thanks, looks great (and thanks to @gbedwell for finding this)!

Mar 11 2019, 8:58 AM · Restricted Project

Mar 8 2019

vsk accepted D58963: [JumpThreading] Retain debug info when replacing branch instructions.

Thanks, lgtm as well.

Mar 8 2019, 11:45 AM · Restricted Project

Mar 1 2019

vsk added a comment to D58726: [DebugInfo][Docs] Explicitly document how dbg.value intrinsics are interpreted in optimized code.

+1, thanks for codifying all of this!

Mar 1 2019, 4:32 PM · Restricted Project

Feb 28 2019

vsk added a comment to D58737: [InstrProf] Use separate comdat group for data and counters.
In D58737#1412846, @rnk wrote:

Oops, forgot to respond to this...

In D58737#1412734, @vsk wrote:

I'm confused by this wording re: comdats in the LangRef: "All global objects that specify this key will only end up in the final object file if the linker chooses that key over some other key". Why can multiple global objects with the same comdat key end up in the final object file?

These words are trying to say that, some number of global objects can use a comdat key. If the object file they are contained in has the prevailing definition of that comdat key, then they will be included, and they will be discarded if not. The prevailing object is typically just the first object file that uses the comdat.

So, multiple objects from *one TU* can use the key and end up in the final binary, but objects from different TUs will never end up mixed together in the final binary.

Feb 28 2019, 10:38 AM · Restricted Project, Restricted Project, Restricted Project

Feb 27 2019

vsk added a comment to D58737: [InstrProf] Use separate comdat group for data and counters.

I'm confused by this wording re: comdats in the LangRef: "All global objects that specify this key will only end up in the final object file if the linker chooses that key over some other key". Why can multiple global objects with the same comdat key end up in the final object file?

Feb 27 2019, 2:40 PM · Restricted Project, Restricted Project, Restricted Project

Feb 26 2019

vsk committed rG73522d167890: [HotColdSplit] Disable splitting for sanitized functions (authored by vsk).
[HotColdSplit] Disable splitting for sanitized functions
Feb 26 2019, 2:55 PM