niravd (Nirav Dave)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Mon, Sep 18

niravd accepted D37987: [DAG] allow store merging before and after legalization (PR34217).

LGTM. It would be nice if we could defer ISel lowering parts occuring during legalization so we can remove the pre-legalization step,

Mon, Sep 18, 1:26 PM

Fri, Sep 8

niravd accepted D37547: [X86] Call removeDeadNode when we're done doing custom isel for mul, div and test.

Yet another case of unused nodes being left behind and causing degradation. We should change the behavior of the DAG ReplaceAllUses-ish functions to check if the old source node becomes unused and if so delete the node. This would obviate the need for this patch and catch the lion share of these sorts of issues but will cause assertions in cases a bunch of cases (which is a better situation than the current silent degradation). I have a first-cut patch (D36643) which fixes just the DAGCombiner CombineTo function and all of the assertion failures from update ordering / double deletions.

Fri, Sep 8, 7:55 PM

Tue, Sep 5

niravd accepted D37464: [X86] Limit store merge size when implicitfloat is enabled (PR34421).

The attributes in the test case can probably be pruned, but otherwise LGTM. Thanks.

Tue, Sep 5, 6:29 AM

Mon, Aug 28

niravd created D37220: [DAG] Bound loop dependence check in merge optimization..
Mon, Aug 28, 10:10 AM

Aug 23 2017

niravd added a comment to D36581: [DAG] Fix Node Replacement in PromoteIntBinOp.

Missed line added. This should be okay. Can I get an LGTM?

Aug 23 2017, 9:08 AM

Aug 11 2017

niravd planned changes to D36643: [DAGCombine] Make CombineTo prune nodes more aggressively..

This is not ready yet, as it needs much more testing to shake out any missing cases. dbabokin should be able to find a number with his testing infrastructure.

Aug 11 2017, 9:23 PM
niravd created D36643: [DAGCombine] Make CombineTo prune nodes more aggressively..
Aug 11 2017, 9:18 PM
niravd updated the diff for D36581: [DAG] Fix Node Replacement in PromoteIntBinOp.

Add missing swap

Aug 11 2017, 7:03 PM
niravd updated the diff for D36581: [DAG] Fix Node Replacement in PromoteIntBinOp.

Add swap when both operands are exapnded loads w/ a dependence.

Aug 11 2017, 12:55 PM
niravd added a comment to D36581: [DAG] Fix Node Replacement in PromoteIntBinOp.

D'oh. It possible though hard. I'll add a cheap check and swap.

Aug 11 2017, 12:54 PM
niravd committed rL310711: Improve handling of insert_subvector of bitcast values.
Improve handling of insert_subvector of bitcast values
Aug 11 2017, 6:23 AM
niravd closed D34571: [DAGCombine] Improve handling of insert_subvector of bitcast values by committing rL310711: Improve handling of insert_subvector of bitcast values.
Aug 11 2017, 6:22 AM
niravd committed rL310710: [X86][DAG] Switch X86 Target to post-legalized store merge.
[X86][DAG] Switch X86 Target to post-legalized store merge
Aug 11 2017, 6:22 AM
niravd closed D34559: [X86][DAG] Switch X86 Target to post-legalized store merge by committing rL310710: [X86][DAG] Switch X86 Target to post-legalized store merge.
Aug 11 2017, 6:22 AM

Aug 10 2017

niravd added a comment to D36581: [DAG] Fix Node Replacement in PromoteIntBinOp.

This is why I hate DAGCombine; you have to constantly worry about the CSE map invalidating pointers you care about, and I'm never confident I'm handling it correctly.

Could we simplify this code using makeEquivalentMemoryOrdering to insert the loads into the right spot in the DAG? Or does that introduce its own RAUW problems?

Aug 10 2017, 8:25 PM
niravd committed rL310659: Revert "[DAG] Cleanup unused nodes after store merge. NFCI.".
Revert "[DAG] Cleanup unused nodes after store merge. NFCI."
Aug 10 2017, 2:04 PM
niravd committed rL310655: [DAG] Relax type restriction for store merge.
[DAG] Relax type restriction for store merge
Aug 10 2017, 12:53 PM
niravd closed D34569: [DAG] Relax type restriction for store merge by committing rL310655: [DAG] Relax type restriction for store merge.
Aug 10 2017, 12:53 PM
niravd committed rL310648: [DAG] Cleanup unused nodes after store merge. NFCI..
[DAG] Cleanup unused nodes after store merge. NFCI.
Aug 10 2017, 11:54 AM
niravd updated the diff for D34569: [DAG] Relax type restriction for store merge.

Excise NFC change and address comments.

Aug 10 2017, 9:08 AM
niravd committed rL310608: [DAG] Rewrite expression. NFC..
[DAG] Rewrite expression. NFC.
Aug 10 2017, 8:30 AM
niravd committed rL310604: [X86] Keep dependencies when constructing loads in combineStore.
[X86] Keep dependencies when constructing loads in combineStore
Aug 10 2017, 8:13 AM
niravd closed D36528: [X86] Keep dependencies when constructing loads in combineStore by committing rL310604: [X86] Keep dependencies when constructing loads in combineStore.
Aug 10 2017, 8:13 AM
niravd created D36581: [DAG] Fix Node Replacement in PromoteIntBinOp.
Aug 10 2017, 8:04 AM

Aug 9 2017

niravd created D36528: [X86] Keep dependencies when constructing loads in combineStore.
Aug 9 2017, 9:17 AM
niravd committed rL310474: [DAG] Explicitly cleanup merged load values during store merge. NFCI..
[DAG] Explicitly cleanup merged load values during store merge. NFCI.
Aug 9 2017, 6:37 AM

Aug 8 2017

niravd abandoned D33518: [AArch64] Fix stores of zero values.
Aug 8 2017, 1:16 PM
niravd updated the diff for D34569: [DAG] Relax type restriction for store merge.

Pull out some NFC changes and use peekThroughBitcast function

Aug 8 2017, 1:11 PM
niravd committed rL310405: [DAG] Introduce peekThroughBitcast function. NFCI..
[DAG] Introduce peekThroughBitcast function. NFCI.
Aug 8 2017, 1:02 PM
niravd committed rL310404: [DAG] Update comments. NFC..
[DAG] Update comments. NFC.
Aug 8 2017, 12:54 PM
niravd accepted D36405: [AsmParser] Hash is not a comment on some targets.

LGTM.

Aug 8 2017, 10:33 AM
niravd added a comment to D36405: [AsmParser] Hash is not a comment on some targets.

Hmm, looking around the only cases I see is that after an assembly label # non-directives are accepted by GCC (here's an example in the wild: https://github.com/android/platform_bionic/blob/master/libc/arch-x86/bionic/__bionic_clone.S). Looking at the parser we do have an explicit check for that in the parser so this should be okay. I also see that at least the arm GCC 4.8.4 errors on hash non-directives at the end of line with various assembly directives / instructions. I suspect there are more test cases that will need to be fixed.

Aug 8 2017, 8:52 AM

Aug 7 2017

niravd requested changes to D36405: [AsmParser] Hash is not a comment on some targets.

LexLineComment will only be called when we # is a asm string line comment or we're at the start of a statement. This change would miss the case of a hash-based (preprocessor) EOL comment trailing a non-empty line which we do need to catch. I believe (but am not 100% certain) that I did find a live assembly case where the # as line comment interpreation for AArch64 was used.

Aug 7 2017, 11:00 AM
niravd committed rL310256: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector..
[DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector.
Aug 7 2017, 7:10 AM
niravd closed D35566: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector. by committing rL310256: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector..
Aug 7 2017, 7:10 AM
niravd committed rL310254: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.
[TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true
Aug 7 2017, 6:56 AM
niravd closed D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true by committing rL310254: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.
Aug 7 2017, 6:56 AM
niravd accepted D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.

LGTM. I'll run it locally myself and land it. Thanks.

Aug 7 2017, 6:35 AM

Aug 4 2017

niravd requested changes to D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.

Based on the UB Error, it seems like the CVT_tied check should be removed and the values of DefaultOffsets shifted over one.

Aug 4 2017, 7:56 PM

Aug 3 2017

niravd committed rL309951: [DAG] Allow merging of stores of vector loads.
[DAG] Allow merging of stores of vector loads
Aug 3 2017, 8:52 AM
niravd closed D36158: [DAG] Allow merging of stores of vector loads by committing rL309951: [DAG] Allow merging of stores of vector loads.
Aug 3 2017, 8:52 AM
niravd committed rL309949: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.
[TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true
Aug 3 2017, 8:41 AM
niravd closed D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true by committing rL309949: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.
Aug 3 2017, 8:41 AM
niravd accepted D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.

Looks Good. I'll just recheck it again and land it.

Aug 3 2017, 8:35 AM

Aug 2 2017

niravd committed rL309830: [DAG] Improve candidate pruning in store merge failure case. NFCI.
[DAG] Improve candidate pruning in store merge failure case. NFCI
Aug 2 2017, 9:36 AM
niravd closed D35901: [DAG] Improve candidate pruning in store merge failure case. NFCI by committing rL309830: [DAG] Improve candidate pruning in store merge failure case. NFCI.
Aug 2 2017, 9:36 AM
niravd updated the diff for D35901: [DAG] Improve candidate pruning in store merge failure case. NFCI.

Cleanup FirstZeroAfterNonZero calc.

Aug 2 2017, 8:15 AM
niravd accepted D36121: Define _GNU_SOURCE for RTEMS c++.

LGTM

Aug 2 2017, 7:33 AM
niravd added a reviewer for D35901: [DAG] Improve candidate pruning in store merge failure case. NFCI: waltl.
Aug 2 2017, 7:28 AM
niravd added a comment to D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.

I suspect that the issue is variation in our cmake configurations. I just rebuilt from a clean build and got the same issues.

Aug 2 2017, 6:49 AM

Aug 1 2017

niravd updated the diff for D34569: [DAG] Relax type restriction for store merge.

Rebase past landed NFC commits.

Aug 1 2017, 7:00 PM
niravd committed rL309777: [DAG] Refactor store merge subexpressions. NFC..
[DAG] Refactor store merge subexpressions. NFC.
Aug 1 2017, 6:09 PM
niravd committed rL309740: [DAG] Factor out common expressions. NFC..
[DAG] Factor out common expressions. NFC.
Aug 1 2017, 1:31 PM
niravd updated the diff for D36158: [DAG] Allow merging of stores of vector loads.

Fold away type modification into landed NFC patch

Aug 1 2017, 11:22 AM
niravd committed rL309719: Pull out VectorNumElements value. NFC..
Pull out VectorNumElements value. NFC.
Aug 1 2017, 11:20 AM
niravd reopened D35566: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector..

Looks like this is raising an assertion. Reopening until I can look at it again.

Aug 1 2017, 11:13 AM
niravd committed rL309717: Revert "[DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector.".
Revert "[DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector."
Aug 1 2017, 11:10 AM
niravd committed rL309708: [DAG] Convert extload check to equivalent type check. NFC..
[DAG] Convert extload check to equivalent type check. NFC.
Aug 1 2017, 10:20 AM
niravd retitled D36158: [DAG] Allow merging of stores of vector loads from [DAG} Allow merging of stores of vector loads to [DAG] Allow merging of stores of vector loads.
Aug 1 2017, 9:20 AM
niravd added a comment to D36158: [DAG] Allow merging of stores of vector loads.

This is factored from D34569

Aug 1 2017, 9:18 AM
niravd created D36158: [DAG] Allow merging of stores of vector loads.
Aug 1 2017, 9:18 AM
niravd committed rL309698: [DAG] Move extload check in store merge. NFC..
[DAG] Move extload check in store merge. NFC.
Aug 1 2017, 9:05 AM
niravd accepted D35979: [X86] Fix a crash in FEntryInserter Pass..

LGTM.

Aug 1 2017, 8:33 AM
niravd added inline comments to D34559: [X86][DAG] Switch X86 Target to post-legalized store merge.
Aug 1 2017, 7:41 AM
niravd updated the diff for D34559: [X86][DAG] Switch X86 Target to post-legalized store merge.

Update test.

Aug 1 2017, 7:41 AM
niravd added a comment to D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.

I'm seeing it after this:

Aug 1 2017, 6:58 AM
niravd committed rL309680: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector..
[DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector.
Aug 1 2017, 6:46 AM
niravd closed D35566: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector. by committing rL309680: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector..
Aug 1 2017, 6:46 AM

Jul 31 2017

niravd updated the diff for D34571: [DAGCombine] Improve handling of insert_subvector of bitcast values.

Undo unrelated updated.

Jul 31 2017, 9:12 AM
niravd updated the diff for D35566: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector..

Unfold unrelated commit.

Jul 31 2017, 9:11 AM
niravd updated the diff for D34571: [DAGCombine] Improve handling of insert_subvector of bitcast values.

Unfold unrelated commit.

Jul 31 2017, 9:09 AM
niravd updated the diff for D35566: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector..

Remove redundant check

Jul 31 2017, 9:05 AM
niravd retitled D35901: [DAG] Improve candidate pruning in store merge failure case. NFCI from [DAG] Improve candidate pruning in store merge failure case. NFC. to [DAG] Improve candidate pruning in store merge failure case. NFCI.
Jul 31 2017, 8:51 AM
niravd requested changes to D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.

I just did a sanity check on this patch upstream before committing and it crashes on a number of AMDGPU tests. Can you take a quick look at what's wrong?

Jul 31 2017, 7:55 AM
niravd updated the diff for D34559: [X86][DAG] Switch X86 Target to post-legalized store merge.

Address comments.

Jul 31 2017, 6:43 AM

Jul 30 2017

niravd added a comment to D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.

Great. It looks like you don't have commit access. Would you like me to commit this patch for you?

Jul 30 2017, 3:13 PM

Jul 28 2017

niravd accepted D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.
Jul 28 2017, 9:16 AM
niravd added inline comments to D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.
Jul 28 2017, 8:07 AM
niravd added a comment to D35998: [TableGen] AsmMatcher: fix OpIdx computation when HasOptionalOperands is true.

Can you add a test case?

Jul 28 2017, 7:26 AM

Jul 27 2017

niravd updated the diff for D34571: [DAGCombine] Improve handling of insert_subvector of bitcast values.

Resolve comments.

Jul 27 2017, 1:24 PM
niravd updated the diff for D34559: [X86][DAG] Switch X86 Target to post-legalized store merge.

Rebase and general update. No real change.

Jul 27 2017, 11:46 AM
niravd updated the diff for D34569: [DAG] Relax type restriction for store merge.

Rework to regularize and simplify bitcasting logic.

Jul 27 2017, 11:32 AM
niravd updated the diff for D35566: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector..

Add type check in truncate case.

Jul 27 2017, 11:29 AM
niravd updated the diff for D35901: [DAG] Improve candidate pruning in store merge failure case. NFCI.

Comment cleanup and apply same logic to final leg of store merge reasoning.

Jul 27 2017, 9:36 AM

Jul 26 2017

niravd updated the diff for D35901: [DAG] Improve candidate pruning in store merge failure case. NFCI.

In addition to alignment, we less restrictive of const zero stores over other constant stores. Add logic to prevent skipping zero cases.

Jul 26 2017, 11:49 AM
niravd planned changes to D35901: [DAG] Improve candidate pruning in store merge failure case. NFCI.
Jul 26 2017, 11:00 AM
niravd created D35901: [DAG] Improve candidate pruning in store merge failure case. NFCI.
Jul 26 2017, 9:40 AM

Jul 25 2017

niravd accepted D35841: [DAG] Move DAGCombiner::GetDemandedBits to SelectionDAG.

LGTM.

Jul 25 2017, 8:26 AM
niravd updated the diff for D35566: [DAG] Extend visitSCALAR_TO_VECTOR optimization to truncated vector..

Update to weaken type restrictions

Jul 25 2017, 8:23 AM

Jul 22 2017

niravd committed rL308833: [DAG] Fix typo preventing some stores merges to truncated stores..
[DAG] Fix typo preventing some stores merges to truncated stores.
Jul 22 2017, 7:09 PM
niravd closed D35623: [DAG] Fix typo preventing some stores merges to truncated stores. by committing rL308833: [DAG] Fix typo preventing some stores merges to truncated stores..
Jul 22 2017, 7:09 PM

Jul 20 2017

niravd committed rL308645: [DAG] Commit missed nit cleanup from r308617. NFC..
[DAG] Commit missed nit cleanup from r308617. NFC.
Jul 20 2017, 11:09 AM
niravd committed rL308618: [DAG] Handle missing transform in fold of value extension case..
[DAG] Handle missing transform in fold of value extension case.
Jul 20 2017, 6:58 AM
niravd closed D35030: [DAG] Handle missing transform in fold of value extension case. by committing rL308618: [DAG] Handle missing transform in fold of value extension case..
Jul 20 2017, 6:58 AM
niravd committed rL308617: [DAG] Optimize away degenerate INSERT_VECTOR_ELT nodes..
[DAG] Optimize away degenerate INSERT_VECTOR_ELT nodes.
Jul 20 2017, 6:50 AM
niravd closed D35563: [DAG] Optimize away degenerate INSERT_VECTOR_ELT nodes. by committing rL308617: [DAG] Optimize away degenerate INSERT_VECTOR_ELT nodes..
Jul 20 2017, 6:50 AM
niravd added a comment to D33518: [AArch64] Fix stores of zero values.

I noticed a few code size regressions looking through the diffs of generated code for this change which were caused by:

  • the offsets of the stores not having the right alignment to allow for stp x opcodes to be formed
  • the offsets of the stores being too large for stp x, but not too large for stp q
  • the offsets of the stores being so large that adds are needed to compute the address for str x, but not so large that adds are needed to compute the address for str q
Jul 20 2017, 6:39 AM

Jul 19 2017

niravd created D35623: [DAG] Fix typo preventing some stores merges to truncated stores..
Jul 19 2017, 8:29 AM

Jul 18 2017

niravd committed rL308350: [DAG] Improve Aliasing of operations to static alloca.
[DAG] Improve Aliasing of operations to static alloca
Jul 18 2017, 1:07 PM
niravd updated the diff for D35030: [DAG] Handle missing transform in fold of value extension case..

Rebase past PR34095 change

Jul 18 2017, 1:00 PM