Page MenuHomePhabricator

niravd (Nirav Dave)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 23 2015, 1:58 PM (194 w, 4 d)

Recent Activity

Wed, Jul 31

niravd accepted D65174: [DAGCombine] Limit the number of times for a store being considered for merging.

LGTM. Thanks!

Wed, Jul 31, 11:55 AM · Restricted Project
niravd added inline comments to D65174: [DAGCombine] Limit the number of times for a store being considered for merging.
Wed, Jul 31, 11:01 AM · Restricted Project
niravd added inline comments to D65174: [DAGCombine] Limit the number of times for a store being considered for merging.
Wed, Jul 31, 6:48 AM · Restricted Project

Tue, Jul 30

niravd added a comment to D65174: [DAGCombine] Limit the number of times for a store being considered for merging.

I havn't got the idea. Could you please elaborate what is the "SDNode *" in the key and what is the "SDNode *" in the value? And how it helps the early return in the dependence check?

Tue, Jul 30, 10:25 AM · Restricted Project
niravd added a comment to D65174: [DAGCombine] Limit the number of times for a store being considered for merging.

It may not be easy for us to prune the searching space without missing some merging opportunity. I feel it is easier to achieve that goal by controlling how many times a {RootNode, StoreNode} pair appear in candidate set repeatedly. With the patch, no bit change in the binaries were found for clang and some large application. So the current patch is a safe change.

Tue, Jul 30, 8:15 AM · Restricted Project

Mon, Jul 29

niravd added a comment to D65174: [DAGCombine] Limit the number of times for a store being considered for merging.

Ah. That's a tricky one. It sounds like this isn't a case of the candidate stores not being mergable, but rather us giving up before we completely verify that.

Mon, Jul 29, 1:18 PM · Restricted Project
niravd added a comment to D65174: [DAGCombine] Limit the number of times for a store being considered for merging.

I check and the reason the candidates cannot be merged isn't because of the legal size. It is that checkMergeStoreCandidatesForDependencies always returns false so https://reviews.llvm.org/D60133 doesn't help.

Mon, Jul 29, 12:02 PM · Restricted Project
niravd added a comment to D65174: [DAGCombine] Limit the number of times for a store being considered for merging.

This has come up a few times before (https://reviews.llvm.org/D60133 is the most recent instance). So far it's either been deemed not important enough or there was a way to limit the candidate search to limit the search to the correct size (e.g. checking the in context max valid store size and making sure there was a valid non-trivial store merge).

Mon, Jul 29, 8:34 AM · Restricted Project

Jul 9 2019

niravd accepted D64205: [X86][SSE] EltsFromConsecutiveLoads - add basic dereferenceable support.

LGTM.

Jul 9 2019, 10:03 AM · Restricted Project

Jul 8 2019

niravd added inline comments to D64205: [X86][SSE] EltsFromConsecutiveLoads - add basic dereferenceable support.
Jul 8 2019, 7:58 AM · Restricted Project

Jul 1 2019

niravd accepted D57302: [DAGCombine] More diamond carry pattern optimization..

LGTM

Jul 1 2019, 6:55 AM · Restricted Project

Jun 25 2019

niravd added a comment to D62890: [DAGCombiner] Merge consecutive stores of vector elements before types are legalized.

It's reasonable assuming we want to prohibit the SystemZ cases. Note that TLI.canMergeConsecutiveStoresOfVectorElements and TLI.canMergeStoresTo are both only called here and should be merged into a single method.

Jun 25 2019, 7:35 AM · Restricted Project

Jun 24 2019

niravd added a comment to D62890: [DAGCombiner] Merge consecutive stores of vector elements before types are legalized.

Cuz PPC's allowsMemoryAccess lacks information about which CombineLevel is at, it fails the check(Only a few vector types are allowed to have misaligned access). As a result, currently no changes happen in PPC's code. I might try to solve this problem with another patch.

Jun 24 2019, 7:09 AM · Restricted Project

Jun 20 2019

niravd added a reviewer for D62890: [DAGCombiner] Merge consecutive stores of vector elements before types are legalized: jonpa.
Jun 20 2019, 1:22 PM · Restricted Project
niravd added a comment to D62890: [DAGCombiner] Merge consecutive stores of vector elements before types are legalized.

Looks like it's mostly an improvement though there are some potential regressions around vector shuffles. I'll leave it to the others if it's acceptable to land.

Jun 20 2019, 7:22 AM · Restricted Project

Jun 18 2019

niravd added a comment to D62890: [DAGCombiner] Merge consecutive stores of vector elements before types are legalized.

By double check, I just meant to look at the results again with isTypeLegal checked, which is where we are?

Jun 18 2019, 8:00 PM · Restricted Project
niravd added a comment to D62890: [DAGCombiner] Merge consecutive stores of vector elements before types are legalized.

Hi, @niravd, after investigate code carefully, I think this check might not be redundant. Considering the case, we have 3 i32 values extracted from vectors, both isTypeLegal and TIL.isTypeLegal see a v3i32 illegal.

Jun 18 2019, 5:53 AM · Restricted Project

Jun 11 2019

niravd added a comment to D57302: [DAGCombine] More diamond carry pattern optimization..

Should we have the corresponding subcarry case folded in here as well?

Jun 11 2019, 9:56 AM · Restricted Project

Jun 10 2019

niravd resigned from D44633: missing FILE symbol for .(s|S) files.
Jun 10 2019, 10:54 AM · Restricted Project

Jun 7 2019

niravd updated subscribers of D62890: [DAGCombiner] Merge consecutive stores of vector elements before types are legalized.
Jun 7 2019, 11:38 AM · Restricted Project

Jun 6 2019

niravd added a comment to D62890: [DAGCombiner] Merge consecutive stores of vector elements before types are legalized.

This seems like it should be folded into the already existing checks. Do you know why NumStoresToMerge was not being before. I expect it's the requirement of a legal types in pre-legal merges or PPC's check for allowed misaligned accesses missing some cases. If it's the former I suspect we can disable the legality requirement prelegaltypes for non-truncated stores (replace TLI.isTypeLegal(ty) with isTypeLegal(ty)).

Jun 6 2019, 9:10 AM · Restricted Project

Jun 5 2019

niravd accepted D62910: [DAGCombine] MergeConsecutiveStores - improve non-temporal load\store handling (PR42123).
Jun 5 2019, 12:12 PM · Restricted Project
niravd added inline comments to D62910: [DAGCombine] MergeConsecutiveStores - improve non-temporal load\store handling (PR42123).
Jun 5 2019, 11:36 AM · Restricted Project
niravd added a comment to D62910: [DAGCombine] MergeConsecutiveStores - improve non-temporal load\store handling (PR42123).

Yes, I think that keeping the flag stuff to an atomic makes the most sense. We should probably make sure we're doing the right thing for the dereferenceable and invariant flags as well.

Jun 5 2019, 8:54 AM · Restricted Project
niravd added a comment to D62910: [DAGCombine] MergeConsecutiveStores - improve non-temporal load\store handling (PR42123).

Don't we need to do something for non-temporal loads as well?

Jun 5 2019, 8:39 AM · Restricted Project

Jun 2 2019

niravd accepted D62633: Recommit r360171: [DAGCombiner] Avoid creating large tokenfactors in visitTokenFactor..

LGTM modulo typo nit. Thanks!

Jun 2 2019, 3:26 PM · Restricted Project

May 30 2019

niravd added a comment to D62633: Recommit r360171: [DAGCombiner] Avoid creating large tokenfactors in visitTokenFactor..

Can you get away with just adding all remaining TFs (not their operands) to Ops? That way we keep the TF smaller and the pruning search still happens. That seems sufficient to fix the dropped operator issue.

Yes we can, kind of. If we just add all remaining TFs, we might get stuck in a loop, because in some cases the simplified TokenFactor == N and N gets added to the worklist, because it is the single user of another tokenfactor. I changed it to expand the first outstanding tokenfactor and add the remaining ones unexpanded. That way, we should not get stuck in a loop, while keeping the size small.

What do you think?

May 30 2019, 7:14 AM · Restricted Project

May 29 2019

niravd added a comment to D62633: Recommit r360171: [DAGCombiner] Avoid creating large tokenfactors in visitTokenFactor..

Can you get away with just adding all remaining TFs (not their operands) to Ops? That way we keep the TF smaller and the pruning search still happens. That seems sufficient to fix the dropped operator issue.

May 29 2019, 4:18 PM · Restricted Project

May 9 2019

niravd accepted D61511: [DAGCombiner] Limit number of nodes explored as store candidates..

LGTM.

May 9 2019, 8:59 AM · Restricted Project

May 7 2019

niravd accepted D61397: [DAGCombiner] Avoid creating large tokenfactors in visitTokenFactor.

LGTM.

May 7 2019, 7:12 AM · Restricted Project

May 6 2019

niravd added a comment to D60133: [DAGCombiner] Improve detection of unmergable stores, based on type size (WIP).

The current interfaces definitely do not provide anything like that. As I recall, most of the use cases requiring canMergeStoresTo involve ad hoc ways to locally declare a type/operation invalid localized to the specific target, so I suggest the just marginally expanding hte interface to capture the one issue we've seen so far (no-implict-float) and defer any deeper analysis for when we run into it.

May 6 2019, 7:31 AM · Restricted Project

May 5 2019

niravd added a comment to D61511: [DAGCombiner] Limit number of nodes explored as store candidates..

This is okay modulo nits, but let's land the TokenFactor operand size limiting first as it also implicitly bounds things.

May 5 2019, 1:38 PM · Restricted Project

May 2 2019

niravd added a comment to D61397: [DAGCombiner] Avoid creating large tokenfactors in visitTokenFactor.

Notes inline, but I think the majority of the compile time improvements come from keepign TokenFactor operand counts bounded. This should be changed to do reflect that.

May 2 2019, 8:20 AM · Restricted Project

Apr 30 2019

niravd accepted D61164: [X86] If PreprocessISelDAG reorders a load before a call, make sure we remove dead nodes from the graph.

LGTM.

Apr 30 2019, 7:16 AM · Restricted Project

Apr 26 2019

niravd added a comment to D61164: [X86] If PreprocessISelDAG reorders a load before a call, make sure we remove dead nodes from the graph.

I think, make it unconditional.

Apr 26 2019, 12:41 PM · Restricted Project
niravd added a comment to D61164: [X86] If PreprocessISelDAG reorders a load before a call, make sure we remove dead nodes from the graph.

I was thinking things pending from the folding optimization in getNode, but I missed that it was the target specific one so it should be fine. That said, I still think it's better to do it unconditionally so it'll be pruned by default with the future additio of Listener-based pruning if that proves too expensive.

Apr 26 2019, 11:43 AM · Restricted Project
niravd added a comment to D61164: [X86] If PreprocessISelDAG reorders a load before a call, make sure we remove dead nodes from the graph.

I worry this is just fix just the dangling nodes as we see them. A brief glance at the AND construction gives us space for this to happen again. I'd much rather we do the removal check unconditionally or leverage something like
DAGUpdateListener to get a more fundamental measure of what nodes may be prunable.

Apr 26 2019, 5:50 AM · Restricted Project

Apr 24 2019

niravd accepted D61048: [X86] Remove dead nodes left after ReplaceAllUsesWith calls during address matching.

DAGCombiner now has infrastructure to strip away nodes that were created but unused in during a visit. It probably makes sense to do something similar in ISel

Apr 24 2019, 8:56 AM · Restricted Project

Apr 18 2019

niravd accepted D60843: [X86] Make sure we copy the HandleSDNode back to N before executing the default code after the switch in matchAddressRecursively.

LGTM.

Apr 18 2019, 6:50 AM · Restricted Project

Apr 2 2019

niravd added a comment to D60133: [DAGCombiner] Improve detection of unmergable stores, based on type size (WIP).

It's not sufficient to check if you can merge two stores into a valid node; there are backends where you need 4 or more to get a legal merged store.

Apr 2 2019, 6:03 PM · Restricted Project

Mar 29 2019

niravd committed rG54f7118de5b9: [DAGCombiner] Rewrite ImproveLifetimeNodeChain to avoid DAG loop. (authored by niravd).
[DAGCombiner] Rewrite ImproveLifetimeNodeChain to avoid DAG loop.
Mar 29 2019, 1:26 PM
niravd committed rL357309: [DAGCombiner] Rewrite ImproveLifetimeNodeChain to avoid DAG loop..
[DAGCombiner] Rewrite ImproveLifetimeNodeChain to avoid DAG loop.
Mar 29 2019, 1:26 PM
niravd abandoned D36643: [DAGCombine] Make CombineTo prune nodes more aggressively..

rL357283 supersedes this.

Mar 29 2019, 12:08 PM
niravd committed rG7e84cacdbd59: [DAG] Avoid redundancy in StoreMerge TokenFactor generation. (authored by niravd).
[DAG] Avoid redundancy in StoreMerge TokenFactor generation.
Mar 29 2019, 11:49 AM
niravd committed rL357299: [DAG] Avoid redundancy in StoreMerge TokenFactor generation..
[DAG] Avoid redundancy in StoreMerge TokenFactor generation.
Mar 29 2019, 11:49 AM
niravd committed rGfe59e14031a9: [DAGCombine] Prune unnused nodes. (authored by niravd).
[DAGCombine] Prune unnused nodes.
Mar 29 2019, 10:36 AM
niravd committed rL357283: [DAGCombine] Prune unnused nodes..
[DAGCombine] Prune unnused nodes.
Mar 29 2019, 10:35 AM
niravd closed D58070: [DAGCombine] Prune unnused nodes..
Mar 29 2019, 10:34 AM · Restricted Project
niravd committed rG610036c50621: [DAG] Set up infrastructure to avoid smart constructor-based dangling nodes (authored by niravd).
[DAG] Set up infrastructure to avoid smart constructor-based dangling nodes
Mar 29 2019, 10:27 AM
niravd committed rL357279: [DAG] Set up infrastructure to avoid smart constructor-based dangling nodes.
[DAG] Set up infrastructure to avoid smart constructor-based dangling nodes
Mar 29 2019, 10:26 AM
niravd closed D58068: [DAG] Set up infrastructure to avoid smart constructor-based dangling nodes.
Mar 29 2019, 10:26 AM · Restricted Project
niravd committed rG9259de217e60: [DAGCombine] Improve Lifetime node chains. (authored by niravd).
[DAGCombine] Improve Lifetime node chains.
Mar 29 2019, 7:10 AM
niravd committed rL357256: [DAGCombine] Improve Lifetime node chains..
[DAGCombine] Improve Lifetime node chains.
Mar 29 2019, 7:10 AM
niravd closed D59795: [DAGCombine] Improve Lifetime node chains..
Mar 29 2019, 7:09 AM · Restricted Project

Mar 28 2019

niravd updated the diff for D58070: [DAGCombine] Prune unnused nodes..

Consider inserted nodes for pruning.

Mar 28 2019, 8:21 PM · Restricted Project
niravd updated the diff for D58068: [DAG] Set up infrastructure to avoid smart constructor-based dangling nodes.

Defer Worklist modification in NodeInserter to follow up patch (D58070).

Mar 28 2019, 8:18 PM · Restricted Project
niravd updated the diff for D59795: [DAGCombine] Improve Lifetime node chains..

Now that lifetime node hashing and TokenFactor cleanup has happened, factor out Dead Store improvements as they are no longer needed to prevent regressions.

Mar 28 2019, 10:55 AM · Restricted Project
niravd committed rG8b9c9822a140: [DAG] Fix Lifetime Node ID hashing. (authored by niravd).
[DAG] Fix Lifetime Node ID hashing.
Mar 28 2019, 8:53 AM
niravd committed rL357179: [DAG] Fix Lifetime Node ID hashing..
[DAG] Fix Lifetime Node ID hashing.
Mar 28 2019, 8:51 AM

Mar 27 2019

niravd committed rG6b741a803867: [DAGCombiner] Teach TokenFactor pruning to peek through lifetime nodes (authored by niravd).
[DAGCombiner] Teach TokenFactor pruning to peek through lifetime nodes
Mar 27 2019, 1:37 PM
niravd committed rL357121: [DAGCombiner] Teach TokenFactor pruning to peek through lifetime nodes.
[DAGCombiner] Teach TokenFactor pruning to peek through lifetime nodes
Mar 27 2019, 1:36 PM
niravd closed D59897: [DAGCombiner] Teach TokenFactor pruning to peek through lifetime nodes.
Mar 27 2019, 1:36 PM · Restricted Project
niravd updated subscribers of D59897: [DAGCombiner] Teach TokenFactor pruning to peek through lifetime nodes.
Mar 27 2019, 1:30 PM · Restricted Project
niravd updated the diff for D59897: [DAGCombiner] Teach TokenFactor pruning to peek through lifetime nodes.

Debug test also changes. h

Mar 27 2019, 1:26 PM · Restricted Project
niravd created D59897: [DAGCombiner] Teach TokenFactor pruning to peek through lifetime nodes.
Mar 27 2019, 1:10 PM · Restricted Project
niravd committed rGc6dfaa0e836c: Revert r356996 "[DAG] Avoid smart constructor-based dangling nodes." (authored by niravd).
Revert r356996 "[DAG] Avoid smart constructor-based dangling nodes."
Mar 27 2019, 12:54 PM
niravd reopened D58068: [DAG] Set up infrastructure to avoid smart constructor-based dangling nodes.

Initial commit reverted due to Halide compile time explosion.

Mar 27 2019, 12:54 PM · Restricted Project
niravd committed rL357116: Revert r356996 "[DAG] Avoid smart constructor-based dangling nodes.".
Revert r356996 "[DAG] Avoid smart constructor-based dangling nodes."
Mar 27 2019, 12:54 PM
niravd updated the diff for D58070: [DAGCombine] Prune unnused nodes..

Address lingering test comments.

Mar 27 2019, 9:17 AM · Restricted Project
niravd committed rGb5630a2ab106: [DAGCombiner] Unify Lifetime and memory Op aliasing. (authored by niravd).
[DAGCombiner] Unify Lifetime and memory Op aliasing.
Mar 27 2019, 7:15 AM
niravd committed rG96a264e053a2: [DAGCombine] Refactor GatherAllAliases. NFCI. (authored by niravd).
[DAGCombine] Refactor GatherAllAliases. NFCI.
Mar 27 2019, 7:15 AM
niravd committed rL357070: [DAGCombiner] Unify Lifetime and memory Op aliasing..
[DAGCombiner] Unify Lifetime and memory Op aliasing.
Mar 27 2019, 7:14 AM
niravd closed D59794: [DAGCombiner] Unify Lifetime and memory Op aliasing..
Mar 27 2019, 7:14 AM · Restricted Project
niravd committed rL357069: [DAGCombine] Refactor GatherAllAliases. NFCI..
[DAGCombine] Refactor GatherAllAliases. NFCI.
Mar 27 2019, 7:14 AM

Mar 26 2019

niravd added inline comments to D59795: [DAGCombine] Improve Lifetime node chains..
Mar 26 2019, 7:13 PM · Restricted Project
niravd closed D58068: [DAG] Set up infrastructure to avoid smart constructor-based dangling nodes.

Landed in rL356996

Mar 26 2019, 6:11 PM · Restricted Project
niravd updated the diff for D59795: [DAGCombine] Improve Lifetime node chains..

Extract copied code into helper and format

Mar 26 2019, 1:37 PM · Restricted Project
niravd added inline comments to D59794: [DAGCombiner] Unify Lifetime and memory Op aliasing..
Mar 26 2019, 1:29 PM · Restricted Project
niravd updated the diff for D59794: [DAGCombiner] Unify Lifetime and memory Op aliasing..

Address comments.

Mar 26 2019, 1:27 PM · Restricted Project
niravd added inline comments to D59794: [DAGCombiner] Unify Lifetime and memory Op aliasing..
Mar 26 2019, 1:27 PM · Restricted Project
niravd updated the diff for D59039: [DAGCombine] Allow GatherAllAliases to pass through inline asm calls and glued nodes..

Revert mips test changes

Mar 26 2019, 8:43 AM · Restricted Project
niravd abandoned D57534: [DAG][ARM] Define upwrapAddress for ARM backend..

This seems like it a natural thing to do as with the X86 backend, but since it doesn't make any test change differences, perhap it is unecessary.

Mar 26 2019, 8:26 AM · Restricted Project
niravd abandoned D53106: [SelectionDAG] Fix behavior topological ordering with regards to glued nodes..

Summarizing offline discussion with jyknight for future reference:

Mar 26 2019, 8:21 AM · Restricted Project
niravd committed rGa28c514581a3: [DAG] Avoid smart constructor-based dangling nodes. (authored by niravd).
[DAG] Avoid smart constructor-based dangling nodes.
Mar 26 2019, 8:09 AM
niravd committed rL356996: [DAG] Avoid smart constructor-based dangling nodes..
[DAG] Avoid smart constructor-based dangling nodes.
Mar 26 2019, 8:07 AM
niravd updated the diff for D58068: [DAG] Set up infrastructure to avoid smart constructor-based dangling nodes.

Rebase to tip before landing.

Mar 26 2019, 7:53 AM · Restricted Project
niravd abandoned D46534: [MC] Update Rootfile in AsmStreamer.

Obsolete.

Mar 26 2019, 7:34 AM · Restricted Project

Mar 25 2019

niravd created D59795: [DAGCombine] Improve Lifetime node chains..
Mar 25 2019, 12:31 PM · Restricted Project
niravd created D59794: [DAGCombiner] Unify Lifetime and memory Op aliasing..
Mar 25 2019, 12:30 PM · Restricted Project
niravd updated the diff for D58070: [DAGCombine] Prune unnused nodes..

Address James' Comments.

Mar 25 2019, 12:09 PM · Restricted Project
niravd updated the diff for D59039: [DAGCombine] Allow GatherAllAliases to pass through inline asm calls and glued nodes..

Check HasSideEffects in analysis for inlineasm.

Mar 25 2019, 12:00 PM · Restricted Project
niravd added inline comments to D59039: [DAGCombine] Allow GatherAllAliases to pass through inline asm calls and glued nodes..
Mar 25 2019, 12:00 PM · Restricted Project
niravd added inline comments to D58070: [DAGCombine] Prune unnused nodes..
Mar 25 2019, 10:51 AM · Restricted Project
niravd added a comment to D58068: [DAG] Set up infrastructure to avoid smart constructor-based dangling nodes.

Ping.

Mar 25 2019, 8:29 AM · Restricted Project
niravd added a comment to D58068: [DAG] Set up infrastructure to avoid smart constructor-based dangling nodes.

Ping/

Mar 25 2019, 8:29 AM · Restricted Project
niravd accepted D59338: MISched: Don't schedule regions with 0 instructions.

LGTM.

Mar 25 2019, 6:38 AM

Mar 18 2019

niravd updated the diff for D58068: [DAG] Set up infrastructure to avoid smart constructor-based dangling nodes.

Address James' comments.

Mar 18 2019, 1:29 PM · Restricted Project
niravd committed rG55c921f4bf34: [DAG] Cleanup unused node in SimplifySelectCC. (authored by niravd).
[DAG] Cleanup unused node in SimplifySelectCC.
Mar 18 2019, 10:02 AM
niravd committed rL356382: [DAG] Cleanup unused node in SimplifySelectCC..
[DAG] Cleanup unused node in SimplifySelectCC.
Mar 18 2019, 10:01 AM
niravd closed D57921: [DAG] Cleanup unused node in SimplifySelectCC..
Mar 18 2019, 10:01 AM · Restricted Project