Page MenuHomePhabricator

tvvikram (Vikram TV)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 10 2015, 9:24 AM (197 w, 5 d)

Recent Activity

Sep 11 2018

tvvikram added a comment to D51153: Break LoopUtils into an Analysis file..

Thank you @dmgreen for the review!

Sep 11 2018, 7:06 PM
tvvikram committed rL342016: Break LoopUtils into an Analysis file..
Break LoopUtils into an Analysis file.
Sep 11 2018, 7:01 PM
tvvikram closed D51153: Break LoopUtils into an Analysis file..
Sep 11 2018, 7:01 PM

Sep 10 2018

tvvikram updated the summary of D51153: Break LoopUtils into an Analysis file..
Sep 10 2018, 8:53 PM
tvvikram updated the diff for D51153: Break LoopUtils into an Analysis file..

Moved Induction and Recurrence descriptors from LoopUtils.h/cpp to IVDescriptors.h/cpp.

Sep 10 2018, 8:48 PM

Sep 9 2018

tvvikram committed rL341776: Move a transformation routine from LoopUtils to LoopVectorize..
Move a transformation routine from LoopUtils to LoopVectorize.
Sep 9 2018, 11:19 PM
tvvikram closed D51837: Move a transformation routine from LoopUtils to LoopVectorize..
Sep 9 2018, 11:19 PM
tvvikram added inline comments to D51837: Move a transformation routine from LoopUtils to LoopVectorize..
Sep 9 2018, 11:07 PM
tvvikram updated the diff for D51837: Move a transformation routine from LoopUtils to LoopVectorize..

Fix file paths. The previous patch missed root directory in its path.

Sep 9 2018, 11:02 PM
tvvikram committed rL341773: Move createMinMaxOp() out of RecurrenceDescriptor..
Move createMinMaxOp() out of RecurrenceDescriptor.
Sep 9 2018, 10:06 PM
tvvikram closed D51838: Move createMinMaxOp() out of RecurrenceDescriptor..
Sep 9 2018, 10:06 PM
tvvikram updated the diff for D51838: Move createMinMaxOp() out of RecurrenceDescriptor..

Try to fix file paths in the diff.
The previous diff somehow missed the root directory in its path.

Sep 9 2018, 9:59 PM
tvvikram added a parent revision for D51838: Move createMinMaxOp() out of RecurrenceDescriptor.: D51153: Break LoopUtils into an Analysis file..
Sep 9 2018, 5:55 AM
tvvikram added a child revision for D51153: Break LoopUtils into an Analysis file.: D51838: Move createMinMaxOp() out of RecurrenceDescriptor..
Sep 9 2018, 5:55 AM
tvvikram created D51838: Move createMinMaxOp() out of RecurrenceDescriptor..
Sep 9 2018, 5:55 AM
tvvikram added a parent revision for D51837: Move a transformation routine from LoopUtils to LoopVectorize.: D51153: Break LoopUtils into an Analysis file..
Sep 9 2018, 12:19 AM
tvvikram added a child revision for D51153: Break LoopUtils into an Analysis file.: D51837: Move a transformation routine from LoopUtils to LoopVectorize..
Sep 9 2018, 12:19 AM
tvvikram created D51837: Move a transformation routine from LoopUtils to LoopVectorize..
Sep 9 2018, 12:19 AM

Sep 5 2018

tvvikram added a comment to D51153: Break LoopUtils into an Analysis file..

Thank you for your inputs.

Sep 5 2018, 9:30 AM
tvvikram added a comment to D51153: Break LoopUtils into an Analysis file..

A gentle ping! Can someone give few basic thoughts about this change?

Sep 5 2018, 2:22 AM

Aug 22 2018

tvvikram added a reviewer for D51153: Break LoopUtils into an Analysis file.: hfinkel.
Aug 22 2018, 11:47 PM
tvvikram created D51153: Break LoopUtils into an Analysis file..
Aug 22 2018, 11:15 PM

Aug 13 2018

tvvikram added a comment to D50167: RFC: [SCEV] Add explicit representations of umin/smin.

For some %r and an indvar %i, the SCEV for (1 + min(r - 1, i)) in smax terms is: (-1 * ((-1 * (zext i32 %r to i64))<nsw> smax {-1,+,-1}<nw><%for.outerloop>))<nsw>.
With your patch, smax gets converted to smin as: (1 + ((-1 + (zext i32 %r to i64))<nsw> smin {0,+,1}<nuw><nsw><%for.outerloop>)), which is correct.
But, it could be simplified further by distributing 1 over smin(?). I might be missing something here. Maybe your patch needs improvement or the getAddExpr() should be improved to handle smin.
P.S.: I am adding 1 explicitly in my code to the min expression i.e. SE->getAddExpr(<scevForMin>, getOne(<ty>)) in both the above cases.

Aug 13 2018, 3:06 AM · Restricted Project

May 8 2018

tvvikram added a comment to D21124: Cache aware Loop Cost Analysis.
In D21124#1091393, @jfb wrote:

llvm-mca doesn't subsume this patch.

Fair. Is this patch going somewhere, or is it abandoned?

May 8 2018, 9:10 AM
tvvikram added a comment to D21124: Cache aware Loop Cost Analysis.

llvm-mca doesn't subsume this patch.

May 8 2018, 2:43 AM

May 3 2018

tvvikram added inline comments to D46201: [DA] Use SCEV to conclude DVEntry::EQ in some cases..
May 3 2018, 4:10 AM

Oct 13 2016

tvvikram added a comment to D21124: Cache aware Loop Cost Analysis.

Vikram, are you still interested in working on this?

Oct 13 2016, 7:59 AM

Sep 14 2016

tvvikram updated subscribers of D24564: [LoopInterchange] Track all dependencies, not just anti dependencies..
Sep 14 2016, 9:44 PM

Jul 6 2016

tvvikram updated subscribers of D19886: Add ability to use DependenceAnalysis from LoopAccessAnalysis.
Jul 6 2016, 9:52 PM

Jun 30 2016

tvvikram added a comment to D21124: Cache aware Loop Cost Analysis.
In D21124#470684, @jfb wrote:

Quick review, nothing technical.

Jun 30 2016, 7:15 PM

Jun 28 2016

tvvikram added reviewers for D21124: Cache aware Loop Cost Analysis: hfinkel, amehsan, jfb, anemet.
Jun 28 2016, 9:39 AM

Jun 15 2016

tvvikram added a reviewer for D17386: Loop Fusion Pass: mssimpso.
Jun 15 2016, 9:16 AM

Jun 13 2016

tvvikram committed rL272545: Fix a typo in loop versioning..
Fix a typo in loop versioning.
Jun 13 2016, 3:56 AM
tvvikram closed D21281: Fix a typo in loop versioning..
Jun 13 2016, 3:56 AM
tvvikram added a reviewer for D21281: Fix a typo in loop versioning.: ashutosh.nema.
Jun 13 2016, 2:02 AM
tvvikram retitled D21281: Fix a typo in loop versioning. from to Fix a typo in loop versioning..
Jun 13 2016, 2:01 AM
tvvikram added a comment to D17386: Loop Fusion Pass.

Vikram,

I remember you mentioning this patch on llvm-dev a while ago, and I believe Adam had given you some preliminary feedback. I'm wondering what your status on this is. Thanks!

Jun 13 2016, 1:17 AM

Jun 11 2016

tvvikram committed rL272479: Delay dominator updation while cloning loop..
Delay dominator updation while cloning loop.
Jun 11 2016, 9:47 AM
tvvikram closed D20899: Delay dominator updation while cloning loop..
Jun 11 2016, 9:47 AM
tvvikram added a comment to D20899: Delay dominator updation while cloning loop..

Thanks!

Jun 11 2016, 9:46 AM

Jun 10 2016

tvvikram added a comment to D20899: Delay dominator updation while cloning loop..

This looks correct to me

Jun 10 2016, 2:42 AM

Jun 9 2016

tvvikram updated D21124: Cache aware Loop Cost Analysis.
Jun 9 2016, 9:36 AM

Jun 8 2016

tvvikram updated subscribers of D20899: Delay dominator updation while cloning loop..
Jun 8 2016, 3:02 AM
tvvikram added a comment to D20899: Delay dominator updation while cloning loop..

Ping!

Jun 8 2016, 3:01 AM
tvvikram updated subscribers of D21124: Cache aware Loop Cost Analysis.
Jun 8 2016, 1:00 AM
tvvikram retitled D21124: Cache aware Loop Cost Analysis from to Cache aware Loop Cost Analysis.
Jun 8 2016, 12:57 AM

Jun 2 2016

tvvikram retitled D20899: Delay dominator updation while cloning loop. from to Delay dominator updation while cloning loop..
Jun 2 2016, 1:05 AM

May 25 2016

tvvikram updated subscribers of D20636: PR26055: Speed up LiveDebugValues::transferDebugValue().
May 25 2016, 11:46 AM

May 24 2016

tvvikram updated subscribers of D20560: [new PM] port LoopAccessAnalysis to new pass manager (part-1).
May 24 2016, 6:39 PM

Feb 18 2016

tvvikram updated subscribers of D17386: Loop Fusion Pass.
Feb 18 2016, 10:32 AM
tvvikram added reviewers for D17386: Loop Fusion Pass: anemet, dberlin.
Feb 18 2016, 9:18 AM
tvvikram retitled D17386: Loop Fusion Pass from to Loop Fusion Pass.
Feb 18 2016, 9:16 AM

Feb 8 2016

tvvikram updated subscribers of D16712: [LoopVersioning] Annotate versioned loop with noalias metadata.
Feb 8 2016, 7:47 PM

Jan 12 2016

tvvikram added a comment to D16039: Use proper dataflow ordering to speed convergence. This will converge the testcase on bug 26055 in 2 iterations. (data structures speedups to come to make even that faster).

Okay!

Jan 12 2016, 7:47 AM

Jan 10 2016

tvvikram added a comment to D16039: Use proper dataflow ordering to speed convergence. This will converge the testcase on bug 26055 in 2 iterations. (data structures speedups to come to make even that faster).

Maybe add an option to enable/disable this pass in this patch itself?

Jan 10 2016, 1:06 AM

Jan 9 2016

tvvikram abandoned D15945: Add option to enable/disable LiveDebugValues pass..

Thanks @dberlin for the suggestions and working on it (http://reviews.llvm.org/D16039)

Jan 9 2016, 11:22 PM

Jan 8 2016

tvvikram added a comment to D15945: Add option to enable/disable LiveDebugValues pass..

Note that test case in https://llvm.org/bugs/show_bug.cgi?id=26055 is produced from auto-generated code, so its BB structure should be very regular, and you can come up with rather precise estimate of how many passes you will need to converge. Does current bound (10) make compile time reasonable for that case?

Yes. The compile time is now almost equal (~0.8 sec on my machine).

Jan 8 2016, 5:37 AM

Jan 7 2016

tvvikram added a comment to D15945: Add option to enable/disable LiveDebugValues pass..

Can we actually add an equivalent clang debug tuning option called "-fvar-tracking" similar to the gcc one here as well?

-eric

Jan 7 2016, 1:57 AM
tvvikram added a comment to D15945: Add option to enable/disable LiveDebugValues pass..

I noticed with a.ll, the test case from https://llvm.org/bugs/show_bug.cgi?id=26055, that there are too many DEBUG_VALUE instructions getting inserted. Statistics showed >6L instructions getting inserted:
616584 live-debug-values - Number of DBG_VALUE instructions inserted

Jan 7 2016, 1:49 AM
tvvikram updated the diff for D15945: Add option to enable/disable LiveDebugValues pass..
Jan 7 2016, 1:49 AM

Jan 6 2016

tvvikram added a comment to D15945: Add option to enable/disable LiveDebugValues pass..
In D15945#321093, @kcc wrote:

please make it false by default since the phase is not really working (compilation times out)

Jan 6 2016, 10:36 PM
tvvikram retitled D15945: Add option to enable/disable LiveDebugValues pass. from to Add option to enable/disable LiveDebugValues pass..
Jan 6 2016, 9:34 PM

Jan 2 2016

tvvikram updated subscribers of D8688: Update MergedLoadStoreMotion to use MemorySSA.
Jan 2 2016, 10:15 AM

Dec 30 2015

tvvikram added a comment to rL256565: [MemoryBuiltins] Delete dead code [NFC].

Can this be reverted? We are using it internally. It is a basic utility anyway.

Dec 30 2015, 10:25 PM

Dec 16 2015

tvvikram closed D11933: Extending debug ranges.

Recommit: rL255759

Dec 16 2015, 8:11 PM
tvvikram abandoned D15574: Remove datalayout and target triple from test cases related to LiveDebugValues pass..
Dec 16 2015, 6:13 PM
tvvikram added a comment to rL255759: Recommit LiveDebugValues pass after fixing a couple of minor issues..

Thanks!

Dec 16 2015, 6:10 PM
tvvikram added a comment to D15574: Remove datalayout and target triple from test cases related to LiveDebugValues pass..

Fixed by chapuni in rL255834. Will abandon this patch.

Dec 16 2015, 6:09 PM
tvvikram added inline comments to D15574: Remove datalayout and target triple from test cases related to LiveDebugValues pass..
Dec 16 2015, 6:04 PM
tvvikram updated the diff for D15574: Remove datalayout and target triple from test cases related to LiveDebugValues pass..

Retained the test cases in X86 folder as they are not generic.

Dec 16 2015, 11:06 AM
tvvikram retitled D15574: Remove datalayout and target triple from test cases related to LiveDebugValues pass. from to Remove datalayout and target triple from test cases related to LiveDebugValues pass..
Dec 16 2015, 9:10 AM
tvvikram committed rL255759: Recommit LiveDebugValues pass after fixing a couple of minor issues..
Recommit LiveDebugValues pass after fixing a couple of minor issues.
Dec 16 2015, 3:13 AM

Dec 13 2015

tvvikram updated subscribers of D7864: This patch introduces MemorySSA, a virtual SSA form for memory. Details on what it looks like are in MemorySSA.h.
Dec 13 2015, 9:26 AM

Dec 11 2015

tvvikram updated the diff for D11933: Extending debug ranges.

I got to know that I had built and tested only for certain targets (sorry that I didn't realize about it earlier). I had to make a couple of minor fixes to make the patch work for all targets.

Dec 11 2015, 10:27 AM

Dec 9 2015

tvvikram added a comment to D11933: Extending debug ranges.

@joker.eph Thanks for reverting.
@jfb I am looking into it.

Dec 9 2015, 3:25 AM

Dec 8 2015

tvvikram added a comment to D11933: Extending debug ranges.

Commit rL255096. Thank you!

Dec 8 2015, 9:59 PM
tvvikram committed rL255096: Implement a new pass - LiveDebugValues - to compute the set of live….
Implement a new pass - LiveDebugValues - to compute the set of live…
Dec 8 2015, 9:52 PM
tvvikram committed rL255095: Test commit access - Fix few missing '.' in comments of LoopInterchange code..
Test commit access - Fix few missing '.' in comments of LoopInterchange code.
Dec 8 2015, 9:19 PM

Dec 4 2015

tvvikram updated the diff for D11933: Extending debug ranges.

Renamed the pass as "LiveDebugValues".

Dec 4 2015, 10:38 AM

Nov 25 2015

tvvikram added a comment to D11933: Extending debug ranges.

I just manually verified that this indeed works as advertised on a couple of old bugreports with debug info + ASAN. Totally awesome!

Good to hear that!

Nov 25 2015, 9:31 AM

Nov 13 2015

tvvikram added inline comments to D11933: Extending debug ranges.
Nov 13 2015, 10:10 AM
tvvikram updated the diff for D11933: Extending debug ranges.

I have made the suggested changes. In few of the test cases I checked using llc --time-passes, the time spent in this pass was <1%.

Nov 13 2015, 10:09 AM

Nov 11 2015

tvvikram added inline comments to D11933: Extending debug ranges.
Nov 11 2015, 3:21 AM
tvvikram updated the diff for D11933: Extending debug ranges.

First, sorry for submitting this after a long gap.

Nov 11 2015, 3:13 AM

Oct 20 2015

tvvikram added a comment to D11933: Extending debug ranges.

Thanks! Any updates?

Oct 20 2015, 1:48 AM

Sep 7 2015

tvvikram added a comment to D11933: Extending debug ranges.

Thanks, more comments inline!

Side note, the clang coding standards recommend to write every comment in the code as complete sentences, with a "." at the end.

Sep 7 2015, 4:24 AM

Sep 4 2015

tvvikram added a comment to D11933: Extending debug ranges.

Ping!

Sep 4 2015, 1:22 AM

Sep 1 2015

tvvikram added a comment to D11933: Extending debug ranges.

Thanks so far. I added a bunch of more detailed comments inline.

I will make the suggested changes and submit a new patch.

I am not sure about the name for the new pass. Please suggest.
I have a couple of names though - ImproveDebugValues.cpp, DebugValueDFA.cpp (DFA: DataFlow Analysis). Please confirm.

To prevent you from having to rename the file and pass more than once, let's defer this decision until the very end.

Thanks.

Another possible name is "DebugValuePropagation", although "LiveDebugValues", or "DebugValueLiveness" would actually be the most fitting, because we are doing a Liveness analysis. This conflicts with the existing LiveDebugVariables pass that consumes all DBG_VALUEs that describe vregs, builds a side-table that is used during register allocation and writes out new DBG_VALUEs with real registers after regalloc is done. Perhaps we could rename the old LiveDebugVariables to "MaterializeDebugValues", or "RegallocDebugValues?

The new pass could be a place to do generic code changes related to DBG_VALUE instructions like inferring multiple locations, inserting missing DBG_VALUE instructions if possible, etc. If that is the case, I think we should name the pass with a generic name. Otherwise, I am okay with the name changes you suggested.

To recap, my vision is that:

  • the old LiveDebugVariables becomes a simple and fast BB-local pass that emits the bare minimum of DBG_VALUEs and
  • this new pass then performs the control-flow-aware liveness DFA that extends their range beyond BB boundaries by inserting enough DBG_VALUEs that
  • the DWARF backend may again assume that DBG_VALUEs are BB-local (and only performs a very simple merging of adjacent ranges).

    This way we will end up with small, understandable, and (via MIR) testable passes with a well-defined scope that do one task well.

Thanks for the high level view.

Sep 1 2015, 9:23 AM
tvvikram updated the diff for D11933: Extending debug ranges.

Made review comment changes.

Sep 1 2015, 9:22 AM

Aug 27 2015

tvvikram added a comment to D11933: Extending debug ranges.

Thanks so far. I added a bunch of more detailed comments inline.

Aug 27 2015, 5:09 AM

Aug 26 2015

tvvikram added a comment to D11933: Extending debug ranges.

Please let me know if the patch requires any more changes.

Aug 26 2015, 9:18 AM
tvvikram added a comment to D11933: Extending debug ranges.

I just commented in the PR:

The debug range extension (http://reviews.llvm.org/D11933) is partially redundant with what LiveDebugVariables is trying to do here. The range extension pass is a full data flow analysis — this means we could simplify LiveDebugVariables and have it no longer attempt to extend the liveness of a DEBUG_VALUE beyond the end of a basic block.

It's possible that simplifying LiveDebugVariables could make up some of the runtime that is spent in the debug range extension pass.

Aug 26 2015, 9:15 AM

Aug 25 2015

tvvikram added a comment to D11933: Extending debug ranges.

I compiled your test case with clang -O1 and here is the input to DBG_VALUE fixup pass (output of StackMapLivenessAnalysis).

The DBG_VALUE instr for "x" is already present in BB#2 and only "x" in RDI got propagated to BB#1.

Looking into this I found that LiveDebugVariables is the culprit here:

void UserValue::extendDef(SlotIndex Idx, unsigned LocNo,
                          LiveRange *LR, const VNInfo *VNI,
                          SmallVectorImpl<SlotIndex> *Kills,
                          LiveIntervals &LIS, MachineDominatorTree &MDT,
                          UserValueScopes &UVS) {
    ...
    for (unsigned i = 0, e = Children.size(); i != e; ++i) {
      MachineBasicBlock *MBB = Children[i]->getBlock();
      if (UVS.dominates(MBB))
        Todo.push_back(LIS.getMBBStartIdx(MBB));
  }
}

It look like it just unconditionally propagates all DBG_VALUEs down the dominator tree, which happens to work fine if there already is another DBG_VALUE but is wrong if the DBG_VALUE is coming from only one of the predecessors like in this example here.

I filed this as PR24563 (https://llvm.org/bugs/show_bug.cgi?id=24563).

Aug 25 2015, 6:12 AM

Aug 23 2015

tvvikram added a comment to D11933: Extending debug ranges.

Ping!

Aug 23 2015, 9:38 PM

Aug 21 2015

tvvikram updated the diff for D11986: Multiple Locations support for Debug Variables.

Please ignore all the previous patches in this review.

Aug 21 2015, 3:12 AM

Aug 20 2015

tvvikram updated the diff for D11933: Extending debug ranges.

Updated .mir test cases and added them under test/DebugInfo/MIR/X86

Aug 20 2015, 10:00 PM
tvvikram updated the diff for D11986: Multiple Locations support for Debug Variables.
Aug 20 2015, 9:56 PM

Aug 19 2015

tvvikram updated the diff for D11933: Extending debug ranges.

Added an .mir test case under test/DebugInfo/MIR/

Aug 19 2015, 12:58 AM
tvvikram added a comment to D11933: Extending debug ranges.

c. Add "STATISTIC" to count the number of DBG_VALUE instructions inserted/deleted. I am not sure what benchmarks to exactly test.

The kind of numbers that I (and Adrian I'm pretty sure) wanted to see are more compiler performance numbers. You add a new pass that will have a cost, and we care a lot about -O0 -g compile times. So what is the cost of your new code in terms of compile time?

Aug 19 2015, 12:46 AM

Aug 18 2015

tvvikram added inline comments to D11933: Extending debug ranges.
Aug 18 2015, 4:35 AM