sanjoy (Sanjoy Das)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 6 2014, 4:30 PM (167 w, 3 d)

Recent Activity

Yesterday

sanjoy committed rL311381: Revert "Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators".
Revert "Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators"
Mon, Aug 21, 1:40 PM
sanjoy closed D36979: Revert "Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators" by committing rL311381: Revert "Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators".
Mon, Aug 21, 1:40 PM
sanjoy created D36979: Revert "Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators".
Mon, Aug 21, 1:27 PM

Sun, Aug 13

sanjoy added inline comments to D36656: [SCCP] Propagate integer range information in IPSCCP (WIP)..
Sun, Aug 13, 3:40 PM

Wed, Aug 9

sanjoy accepted D36552: [LVI] Fix LVI compile time regression around constantFoldUser().

lgtm

Wed, Aug 9, 6:33 PM
sanjoy added inline comments to D36552: [LVI] Fix LVI compile time regression around constantFoldUser().
Wed, Aug 9, 4:09 PM
sanjoy requested changes to D36552: [LVI] Fix LVI compile time regression around constantFoldUser().
Wed, Aug 9, 2:53 PM

Tue, Aug 8

sanjoy accepted D36392: [ImplicitNullCheck] Fix the bug when dependent instruction accesses memory.

lgtm again

Tue, Aug 8, 6:11 PM
sanjoy added a comment to D36442: [DomTree] Use a non-recursive DFS instead of a recursive one; NFC.

LGTM.

Out of curiosity: what kind of IR triggers stack overflow here? I'd suspect it would be have to a very large and nested loop.

Tue, Aug 8, 10:20 AM
sanjoy committed rL310383: [DomTree] Use a non-recursive DFS instead of a recursive one; NFC.
[DomTree] Use a non-recursive DFS instead of a recursive one; NFC
Tue, Aug 8, 10:18 AM
sanjoy closed D36442: [DomTree] Use a non-recursive DFS instead of a recursive one; NFC by committing rL310383: [DomTree] Use a non-recursive DFS instead of a recursive one; NFC.
Tue, Aug 8, 10:17 AM

Mon, Aug 7

sanjoy created D36442: [DomTree] Use a non-recursive DFS instead of a recursive one; NFC.
Mon, Aug 7, 9:51 PM
sanjoy accepted D36392: [ImplicitNullCheck] Fix the bug when dependent instruction accesses memory.

lgtm with minor comments

Mon, Aug 7, 9:36 AM

Fri, Aug 4

sanjoy accepted D35256: [SCEV] Try harder to preserve NSW information for sext(sub) expressions.

lgtm!

Fri, Aug 4, 10:10 AM

Thu, Aug 3

sanjoy added inline comments to D36247: [LVI] Constant-propagate a zero extension of the switch condition value through case edges.
Thu, Aug 3, 1:53 PM
sanjoy accepted D36247: [LVI] Constant-propagate a zero extension of the switch condition value through case edges.

lgtm

Thu, Aug 3, 12:40 PM

Wed, Aug 2

sanjoy added a comment to D36247: [LVI] Constant-propagate a zero extension of the switch condition value through case edges.

Code wise this looks correct to me, but I'd prefer to phrase the solution a bit differently in the commit message and in the comments: IIUC the bug we had was that we were assuming Condition != T (in the default block) implied f(Condition) != f(T). However this isn't true for a non injective f (like f(i32 x) = and i32 x, 2).

Wed, Aug 2, 5:40 PM
sanjoy accepted D36087: [SCEV] Re-enable "Cache results of computeExitLimit".

I've also verified that this does not break on the internal test case we had.

Wed, Aug 2, 11:55 AM
sanjoy added inline comments to D35256: [SCEV] Try harder to preserve NSW information for sext(sub) expressions.
Wed, Aug 2, 11:46 AM
sanjoy requested changes to D35989: [SCEV][NFC] Introduces expression sizes estimation.

Requesting changes as per previous comments.

Wed, Aug 2, 10:40 AM

Tue, Aug 1

sanjoy accepted D36188: [PM] Fix a bug where through CGSCC iteration we can get infinite-inlining across multiple runs of the inliner by keeping a tiny history of internal-to-SCC inlining decisions..
Tue, Aug 1, 6:39 PM
sanjoy committed rL309758: [SCEV/IndVars] Always compute loop exiting values if the backedge count is 0.
[SCEV/IndVars] Always compute loop exiting values if the backedge count is 0
Tue, Aug 1, 3:38 PM

Mon, Jul 31

sanjoy requested changes to D35256: [SCEV] Try harder to preserve NSW information for sext(sub) expressions.
Mon, Jul 31, 10:09 AM
sanjoy accepted D30703: [DSE] Merge stores when the later store only writes to memory locations the early store also wrote to..

LGTM

Mon, Jul 31, 10:00 AM

Sat, Jul 29

sanjoy requested changes to D35256: [SCEV] Try harder to preserve NSW information for sext(sub) expressions.

Sorry for the delay, this one somehow slipped through the cracks.

Sat, Jul 29, 10:32 PM

Fri, Jul 28

sanjoy committed rL309480: [SCEV] Change an early exit to an assert; NFC.
[SCEV] Change an early exit to an assert; NFC
Fri, Jul 28, 10:33 PM
sanjoy added a comment to D35989: [SCEV][NFC] Introduces expression sizes estimation.

I'm worried about the peak memory usage impact of this change. Can you try to do a clang bootstrap and count the maximum number of SCEV objects alive at any one given time? That information would also be generally useful.

Fri, Jul 28, 5:09 PM
sanjoy added inline comments to D34200: [PM/unswitch] Teach SimpleLoopUnswitch to do non-trivial unswitching, making it no longer even remotely simple..
Fri, Jul 28, 2:20 PM

Thu, Jul 27

sanjoy added inline comments to D35931: [SCEV] Do not visit nodes twice in containsConstantSomewhere.
Thu, Jul 27, 10:32 PM
sanjoy accepted D35931: [SCEV] Do not visit nodes twice in containsConstantSomewhere.

This LGTM, but I think the use site is using too large of a hammer. It only cares about binary add instructions in which one of the operands is a multiplication with a constant or a constant. We should write a check for just that, instead of traversing the whole expression like this.

Thu, Jul 27, 9:46 PM
sanjoy committed rL309357: Revert "[SCEV] Cache results of computeExitLimit".
Revert "[SCEV] Cache results of computeExitLimit"
Thu, Jul 27, 8:25 PM
sanjoy added inline comments to D35931: [SCEV] Do not visit nodes twice in containsConstantSomewhere.
Thu, Jul 27, 3:09 PM
sanjoy added inline comments to D35931: [SCEV] Do not visit nodes twice in containsConstantSomewhere.
Thu, Jul 27, 12:39 AM

Wed, Jul 26

sanjoy accepted D34822: [LVI] Constant-propagate a zero extension of the switch condition value through case edges.

lgtm with minor comments.

Wed, Jul 26, 4:56 PM

Tue, Jul 25

sanjoy committed rL309072: [SCEV] Remove unnecessary call to forgetMemoizedResults.
[SCEV] Remove unnecessary call to forgetMemoizedResults
Tue, Jul 25, 6:33 PM
sanjoy added inline comments to D34822: [LVI] Constant-propagate a zero extension of the switch condition value through case edges.
Tue, Jul 25, 5:14 PM

Jul 21 2017

sanjoy accepted D35758: [LIR] Teach LIR to avoid extending the BE count prior to adding one to it when safe..

lgtm!

Jul 21 2017, 8:28 PM
sanjoy added a comment to D35664: [SCEV] Limit max size of AddRecExpr during evolving.

Hi Sanjoy,

Investigating the impact of thresholds may be an interesting idea indeed. But it is not clear where should we measure them. This patch came from evaluation of bugs 33753 and 33494 (and actually is a fix for the 1st one and some subset of tests of the latter). The problem is that the impact of these parameters can differ dramatically on real workloads and fuzzed tests of special forms. For example, some of the tests from bug 33494 pass with only limit for AddRecs set, and others also need reduction of Arith limit to pass in reasonable time. The values of those limits are chosen absolutely randomly, though, and it is not clear what we should use as a reference when chosing their values.

Jul 21 2017, 3:09 PM
sanjoy accepted D35664: [SCEV] Limit max size of AddRecExpr during evolving.

This change itself looks fine to me. However, I'm getting a bit worried about the arbitrariness of thresholds we have now. Can you figure out some way to get the histograms of these limits seen on a run (and perhaps run it over some benchmarks you have)? That is, what are the values of AddRec->getNumOperands() + OtherAddRec->getNumOperands() - 1 we actually see in a run? We could record and report this dependent on a flag.

Jul 21 2017, 10:42 AM

Jul 20 2017

sanjoy requested changes to D34822: [LVI] Constant-propagate a zero extension of the switch condition value through case edges.
Jul 20 2017, 10:55 AM

Jul 19 2017

sanjoy accepted D35609: [LICM] Make sinkRegion and hoistRegion non-recursive.
Jul 19 2017, 6:35 PM
sanjoy added inline comments to D35609: [LICM] Make sinkRegion and hoistRegion non-recursive.
Jul 19 2017, 6:25 PM
sanjoy added inline comments to D35609: [LICM] Make sinkRegion and hoistRegion non-recursive.
Jul 19 2017, 10:03 AM

Jul 18 2017

sanjoy added inline comments to D35609: [LICM] Make sinkRegion and hoistRegion non-recursive.
Jul 18 2017, 11:14 PM

Jul 17 2017

sanjoy accepted D35010: [IRCE] Recognize loops with ne/eq latch conditions.
Jul 17 2017, 5:00 PM
sanjoy accepted D35499: [InstCombine] Simplify pointer difference subtractions (GEP-GEP) where GEPs have other uses and one non-constant index.

lgtm with minor nits

Jul 17 2017, 2:44 PM

Jul 16 2017

sanjoy added a comment to D34029: Infer lowest bits of an integer Multiply when the low bits of the operands are known.

@sanjoy , can you please take a look at this? I recall this coming up as something we may not be able to do in the face of undef, etc.

Jul 16 2017, 2:38 PM

Jul 15 2017

sanjoy added inline comments to D35347: [IRCE] Fix corner case with Start = INT_MAX.
Jul 15 2017, 4:57 PM
sanjoy requested changes to D35010: [IRCE] Recognize loops with ne/eq latch conditions.

Some comments / questions inline.

Jul 15 2017, 4:00 PM
sanjoy accepted D30041: [PSCEV] Create AddRec for Phis in cases of possible integer overflow, using runtime checks.

lgtm with one nit

Jul 15 2017, 12:33 PM

Jul 14 2017

sanjoy added a comment to D35439: Make promoteLoopAccessesToScalars independent of AliasSet [NFC].

Actually, one good reason for switching to passing in a vector of pointers to promoteLoopAccessesToScalars is that we probably (?) want a deterministic order of insertion into LoopUses since that will influence what LoopPromoter does.

Jul 14 2017, 7:59 PM
sanjoy added inline comments to D35439: Make promoteLoopAccessesToScalars independent of AliasSet [NFC].
Jul 14 2017, 7:16 PM

Jul 12 2017

sanjoy requested changes to D34200: [PM/unswitch] Teach SimpleLoopUnswitch to do non-trivial unswitching, making it no longer even remotely simple..

Second and last part of the review.

Jul 12 2017, 10:16 PM

Jul 11 2017

sanjoy added a comment to D35227: [LV] Don't allow outside uses of IVs if the SCEV is predicated on loop conditions.

I'm probably missing something obvious here. For external IV uses, we compute the end IV value assuming we're coming from the vector loop. But if we executed the vector loop, shouldn't the SCEV predicate have been true, which would mean that the PSEV was a safe assumption?

You're not necessarily missing something obvious, I may be misunderstanding what PSCEV does here. If I understand correctly, PSCEV adds the assumption the IV doesn't overflow inside the loop. In this case, what I see is that it overflows on loop exit. So, either:
a) I misunderstand what PSCEV is doing here. :-)
b) PSCEV is wrong inside the loop.
c) The assumption PSCEV makes is correct, except on exit. If this is the case, then we can't use the SCEV we get to compute the value after the last iteration.

Jul 11 2017, 9:04 PM
sanjoy requested changes to D30041: [PSCEV] Create AddRec for Phis in cases of possible integer overflow, using runtime checks.

Mostly minor stuff.

Jul 11 2017, 4:04 PM

Jul 10 2017

sanjoy added a comment to D21723: [RFC] Enhance synchscope representation.

langref changes lgtm

Jul 10 2017, 10:30 PM

Jul 9 2017

sanjoy requested changes to D35010: [IRCE] Recognize loops with ne/eq latch conditions.

Did you consider a somewhat more general trick of using the loop's BE taken count to derive an equivalent BE condition? E.g. if the loops be taken count is 100, and the post-incremented indvar (the one the BE condition uses) is {5,+,1} (everything is i32) then the BE taken condition can be ++I s< 105 (and you can derive this without looking at whether the BE condition was expressed using SLT or NE or EQ etc.).

Jul 9 2017, 6:14 AM

Jul 4 2017

sanjoy added a comment to D34200: [PM/unswitch] Teach SimpleLoopUnswitch to do non-trivial unswitching, making it no longer even remotely simple..

In this episode I've reviewed buildClonedLoops and cloneLoopNest.

Jul 4 2017, 10:18 PM
sanjoy accepted D34979: [IndVars] Canonicalize comparisons between non-negative values and indvars.

lgtm

Jul 4 2017, 1:15 PM

Jul 3 2017

sanjoy accepted D34207: [IndVarSimplify] Add AShr exact flags using induction variables ranges..
Jul 3 2017, 1:52 PM
sanjoy accepted D34598: ScalarEvolution: Add URem support.
Jul 3 2017, 12:33 PM

Jun 30 2017

sanjoy accepted D34583: [LSR] Narrow search space by filtering non-optimal formulae with the same ScaledReg and Scale..

lgtm

Jun 30 2017, 4:55 PM

Jun 29 2017

sanjoy added inline comments to D34583: [LSR] Narrow search space by filtering non-optimal formulae with the same ScaledReg and Scale..
Jun 29 2017, 5:53 PM
sanjoy requested changes to D34583: [LSR] Narrow search space by filtering non-optimal formulae with the same ScaledReg and Scale..
Jun 29 2017, 3:39 PM

Jun 25 2017

sanjoy added a comment to D34200: [PM/unswitch] Teach SimpleLoopUnswitch to do non-trivial unswitching, making it no longer even remotely simple..

I did a quick scan and have a couple of low-level comments inline.

Jun 25 2017, 4:37 PM

Jun 24 2017

sanjoy accepted D33110: [CodeGenPrepare] Don't create inttoptr for ni ptrs.

lgtm

Jun 24 2017, 9:41 PM
sanjoy accepted D32978: [SCEV] Avoid copying ConstantRange just to get the min/max value.

I did not carefully check the getSignedRange(RHS).getSignedMin() -> getSignedRangeMin(RHS) -like transforms, I assumed you got those right.

Jun 24 2017, 1:21 PM
sanjoy accepted D31954: [InstCombine] Retain TBAA when narrowing loads.

lgtm

Jun 24 2017, 12:48 PM
sanjoy resigned from D22298: [LCG] Update and expand comments to properly document the design motivation, tradeoffs, and constraints..

Inactive, as far as I can tell.

Jun 24 2017, 12:41 PM
sanjoy resigned from D17203: [LICM] Sink entire inner loops..

Inactive, as far as I can tell.

Jun 24 2017, 12:41 PM
sanjoy resigned from D21041: [GVN] PRE can't hoist loads across calls in general..

Inactive, as far as I can tell.

Jun 24 2017, 12:38 PM
sanjoy resigned from D21010: Replace the implementation of ConstantRange::binaryAnd..

Inactive, as far as I can tell.

Jun 24 2017, 12:38 PM
sanjoy resigned from D29965: PR18606 fix hang in GetMinTrailingZeros.

Inactive, as far as I can tell.

Jun 24 2017, 12:37 PM
sanjoy requested changes to D33332: Possible typo in ProgrammersManual documentation.

I'm not sure that this is a typo -- the existing doc is probably talking about the shapes of the struct and array types.

Jun 24 2017, 12:37 PM

Jun 22 2017

sanjoy abandoned D33300: [ImplicitNullChecks] Uphold an invariant in areMemoryOpsAliased.

@skatkov has checked in a fix for this issue.

Jun 22 2017, 11:53 PM

Jun 21 2017

sanjoy added inline comments to D33850: Inlining: Don't re-map simplified cloned instructions..
Jun 21 2017, 11:24 PM

Jun 20 2017

sanjoy accepted D34385: [ImplicitNullChecks] Uphold an invariant in areMemoryOpsAliased.

lgtm

Jun 20 2017, 10:35 PM
sanjoy accepted D34397: [SCEV] Make MulOpsInlineThreshold lower to avoid excessive compilation time.

What about the old TODO item of extending powi to integer types and folding them together in the middle end?

Anyways, even if we have a concept of powi in IR, it does not automatically mean that we have it in SCEV. It would be extremely good to have it, though, but it looks like a massive task that takes time.

Jun 20 2017, 10:29 PM
sanjoy added a comment to D34397: [SCEV] Make MulOpsInlineThreshold lower to avoid excessive compilation time.

32 seems like a sane default value, but where exactly do we get stuck here? If there are O(N^2) or O(N^3) algorithms that are not super difficult to remove, we should just remove them instead of working around them like this.

Jun 20 2017, 10:28 PM
sanjoy requested changes to D34207: [IndVarSimplify] Add AShr exact flags using induction variables ranges..
Jun 20 2017, 10:03 AM

Jun 19 2017

sanjoy added a comment to D33865: Mark llvm.*annotation intrinsics as NoMem and Speculatable.

Do we need to add more reviewers?

JFYI: so far you haven't added any reviewers. :)

However, if you want to make this change, at the very least you also need to edit the language reference entry for llvm.var.annotation to state that it isn't allowed to express control flow dependent properties. Can you also please give some examples of how you use llvm.var.annotate (for discussion)?

I use llvm.annotation to attach an integer number to some loads and stores to identify them uniquely. For loads, I annotate the return value, i.e. llvm.annotation(load(pointer), number). For stores, I annotate the input value being stored, i.e. store(llvm.annotation(value, number)). The load and store offsets can be rather arbitrary and not meaningful in my case (graphics shaders), but the numbers are unique. Then I run LLVM IR optimizations (DCE, CSE, etc.), and then I walk the IR and look at llvm.annotation(.., number) to see which loads and stores survived DCE. Since the loads and stores are from "invisible" memory, CSE can be used, but I need IntrNoMem and IntrSpeculatable there, so that llvm.annotation itself doesn't prevent CSE from being done.

Jun 19 2017, 10:19 PM
sanjoy committed rL305756: Fix machine instruction in test case.
Fix machine instruction in test case
Jun 19 2017, 3:36 PM
sanjoy updated the diff for D30445: [ValueTracking] Add a isIVNeverPoison helper.
Jun 19 2017, 11:30 AM

Jun 17 2017

sanjoy accepted D34323: Add ArgMemOnly attribute to strlen and wcslen, i.e. they only read memory (string) passed to them..

lgtm

Jun 17 2017, 8:00 PM
sanjoy requested changes to D34323: Add ArgMemOnly attribute to strlen and wcslen, i.e. they only read memory (string) passed to them..

Generally looks good, but the test isn't testing anything.

Jun 17 2017, 7:09 PM
sanjoy added inline comments to D30446: [IndVars] Do not branch on poison.
Jun 17 2017, 3:45 PM
sanjoy added inline comments to D30445: [ValueTracking] Add a isIVNeverPoison helper.
Jun 17 2017, 3:17 PM
sanjoy added inline comments to D30443: Add a propagateKnownNonPoison helper.
Jun 17 2017, 3:17 PM
sanjoy committed rL305639: [SROA] Add support for non-integral pointers.
[SROA] Add support for non-integral pointers
Jun 17 2017, 1:28 PM
sanjoy closed D32203: [SROA] Add support for non-integral pointers by committing rL305639: [SROA] Add support for non-integral pointers.
Jun 17 2017, 1:28 PM
sanjoy added a comment to D33865: Mark llvm.*annotation intrinsics as NoMem and Speculatable.

Do we need to add more reviewers?

Jun 17 2017, 1:28 PM

Jun 16 2017

sanjoy accepted D34025: [SCEV] Teach SCEVExpander to expand BinPow.

lgtm with nits

Jun 16 2017, 9:04 PM

Jun 14 2017

sanjoy accepted D33984: [ScalarEvolution] Apply Depth limit to getMulExpr.

lgtm

Jun 14 2017, 11:04 PM

Jun 11 2017

sanjoy accepted D33466: Fix file permissions.

You're just making the file non-executable, right? If so, LGTM!

Jun 11 2017, 9:56 PM
sanjoy added a comment to D33839: Prevent outlining of basicblock that uses BlockAddress.

The spec probably needs update, but I remember inliner or function cloning does something to avoid this. +cc Chandler for comments.

Jun 11 2017, 9:48 PM

Jun 10 2017

sanjoy added a comment to D34025: [SCEV] Teach SCEVExpander to expand BinPow.

Sanjoy, what is the difference between having one SCEV with exponential number of operands or having a SCEV tree with 2 operands in each node, but the same number of operands in total? We won't win anything doing so.

Jun 10 2017, 12:47 PM

Jun 9 2017

sanjoy requested changes to D33984: [ScalarEvolution] Apply Depth limit to getMulExpr.
Jun 9 2017, 2:48 PM
sanjoy requested changes to D34025: [SCEV] Teach SCEVExpander to expand BinPow.

Hi Maxim,

Jun 9 2017, 2:36 PM

Jun 7 2017

sanjoy accepted D33979: [IndVars] Add an option to be able to disable LFTR.

This LGTM.

Jun 7 2017, 11:59 AM

Jun 5 2017

sanjoy added inline comments to D21723: [RFC] Enhance synchscope representation.
Jun 5 2017, 1:04 PM