niravd (Nirav Dave)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 23 2015, 1:58 PM (112 w, 2 d)

Recent Activity

Yesterday

niravd added inline comments to D42128: [X86] Extend load-op-store fusion merge to ADC/SBB..
Wed, Jan 17, 12:10 PM
niravd added a dependent revision for D42128: [X86] Extend load-op-store fusion merge to ADC/SBB.: D41293: [DAG,X86] Improve Dependency analysis when doing multi-node Instruction Selection.
Wed, Jan 17, 12:10 PM
niravd added a dependency for D41293: [DAG,X86] Improve Dependency analysis when doing multi-node Instruction Selection: D42128: [X86] Extend load-op-store fusion merge to ADC/SBB..
Wed, Jan 17, 12:10 PM
niravd updated the diff for D42128: [X86] Extend load-op-store fusion merge to ADC/SBB..

Add ADC immediate tests. I haven't worked out a way to the appropriate SUB immediate nodes via LLVM IR as they're realized via add.

Wed, Jan 17, 12:08 PM

Tue, Jan 16

niravd updated the diff for D41293: [DAG,X86] Improve Dependency analysis when doing multi-node Instruction Selection.

Rebasing past D42128.

Tue, Jan 16, 1:08 PM
niravd created D42128: [X86] Extend load-op-store fusion merge to ADC/SBB..
Tue, Jan 16, 1:07 PM

Fri, Jan 12

niravd updated the diff for D41293: [DAG,X86] Improve Dependency analysis when doing multi-node Instruction Selection.

In testing I discovered the follow on patch to X86's load-op-store pattern check needed to be folded in. Added discovered test as well.

Fri, Jan 12, 11:16 AM

Thu, Jan 11

niravd updated the diff for D41293: [DAG,X86] Improve Dependency analysis when doing multi-node Instruction Selection.

Minor cleanup.

Thu, Jan 11, 9:27 AM

Wed, Jan 10

niravd updated the diff for D41293: [DAG,X86] Improve Dependency analysis when doing multi-node Instruction Selection.

Address comments and rebase.

Wed, Jan 10, 1:43 PM

Tue, Jan 9

niravd committed rL322085: [DAG] Elide overlapping stores.
[DAG] Elide overlapping stores
Tue, Jan 9, 7:24 AM

Mon, Jan 8

niravd added inline comments to D41235: [DAGCOmbine] Ensure that (brcond (setcc ...)) is handled in a canonical manner..
Mon, Jan 8, 9:35 AM
niravd added inline comments to D41505: [DAG] Teach findBaseOffset to interpret indexes of indexed memory operations.
Mon, Jan 8, 8:40 AM
niravd committed rL322003: [DAG] Teach BaseIndexOffset to correctly handle with indexed operations.
[DAG] Teach BaseIndexOffset to correctly handle with indexed operations
Mon, Jan 8, 8:22 AM
niravd closed D41701: [DAG] Teach BaseIndexOffset to correctly handle with indexed operations.
Mon, Jan 8, 8:22 AM

Fri, Jan 5

niravd added inline comments to D41701: [DAG] Teach BaseIndexOffset to correctly handle with indexed operations.
Fri, Jan 5, 11:38 AM
niravd added a comment to D41701: [DAG] Teach BaseIndexOffset to correctly handle with indexed operations.

I would prefer to have a regression test, but it's okay if you can't come up with a way to write one.

Fri, Jan 5, 10:51 AM
niravd updated the diff for D41701: [DAG] Teach BaseIndexOffset to correctly handle with indexed operations.

Correct failing on non-constant indices.

Fri, Jan 5, 10:44 AM
niravd added inline comments to D41293: [DAG,X86] Improve Dependency analysis when doing multi-node Instruction Selection.
Fri, Jan 5, 8:42 AM

Wed, Jan 3

niravd updated the diff for D41293: [DAG,X86] Improve Dependency analysis when doing multi-node Instruction Selection.

Rebase and fix NodeId pruning to also ignore 0 ids (from legalization) and turn pruning on for checks only

Wed, Jan 3, 10:25 AM
niravd updated the diff for D41505: [DAG] Teach findBaseOffset to interpret indexes of indexed memory operations.

Factored out the indexed memory operation correctness check into D417101.

Wed, Jan 3, 7:53 AM
niravd added a dependent revision for D41701: [DAG] Teach BaseIndexOffset to correctly handle with indexed operations: D41505: [DAG] Teach findBaseOffset to interpret indexes of indexed memory operations.
Wed, Jan 3, 7:46 AM
niravd added a dependency for D41505: [DAG] Teach findBaseOffset to interpret indexes of indexed memory operations: D41701: [DAG] Teach BaseIndexOffset to correctly handle with indexed operations.
Wed, Jan 3, 7:46 AM
niravd created D41701: [DAG] Teach BaseIndexOffset to correctly handle with indexed operations.
Wed, Jan 3, 7:46 AM

Fri, Dec 22

niravd committed rL321391: [DAG] Add missing case check from findbaseoffset merge from r321389..
[DAG] Add missing case check from findbaseoffset merge from r321389.
Fri, Dec 22, 2:07 PM
niravd committed rL321389: Integrate findBaseOffset address analyses to BaseIndexOffset. NFCI..
Integrate findBaseOffset address analyses to BaseIndexOffset. NFCI.
Fri, Dec 22, 1:21 PM
niravd committed rL321380: Revert "[DAG] Integrate findBaseOffset address analyses to BaseIndexOffset..
Revert "[DAG] Integrate findBaseOffset address analyses to BaseIndexOffset.
Fri, Dec 22, 11:35 AM
niravd updated the diff for D41505: [DAG] Teach findBaseOffset to interpret indexes of indexed memory operations.

clang-format and fix missing check that we're using EA output in address calc.

Fri, Dec 22, 9:31 AM
niravd committed rL321364: [DAG] Integrate findBaseOffset address analyses to BaseIndexOffset. NFCI..
[DAG] Integrate findBaseOffset address analyses to BaseIndexOffset. NFCI.
Fri, Dec 22, 9:00 AM

Thu, Dec 21

niravd created D41505: [DAG] Teach findBaseOffset to interpret indexes of indexed memory operations.
Thu, Dec 21, 11:00 AM

Wed, Dec 20

niravd committed rL321204: [DAG] Fix condition on overlapping store check..
[DAG] Fix condition on overlapping store check.
Wed, Dec 20, 11:07 AM
niravd accepted D41350: [DAGCombine] Improve ReduceLoadWidth for SRL.

LGTM modulo rksimon's comment.

Wed, Dec 20, 8:51 AM

Tue, Dec 19

niravd committed rL321089: [DAG] Elide overlapping store.
[DAG] Elide overlapping store
Tue, Dec 19, 9:11 AM
niravd closed D40969: [DAG] Elide overlapping stores.
Tue, Dec 19, 9:11 AM

Dec 18 2017

niravd retitled D41293: [DAG,X86] Improve Dependency analysis when doing multi-node Instruction Selection from [DAG] Improve multi-chain merging in ISel to [DAG] Improve Dependency analysis when doing multi-node Instruction Selection.
Dec 18 2017, 1:41 PM
niravd added a comment to D40969: [DAG] Elide overlapping stores.

Ping.

Dec 18 2017, 7:47 AM
niravd added a comment to D41350: [DAGCombine] Improve ReduceLoadWidth for SRL.

All of the current tests look good to me. I think you should add some tests for non-byte shifts.

Dec 18 2017, 7:29 AM

Dec 15 2017

niravd accepted D41177: [DAGCombine] Move AND nodes to multiple load leaves.

Do you have a reasonable test case of the failing case you can add?

Dec 15 2017, 9:22 AM
niravd added a comment to D41177: [DAGCombine] Move AND nodes to multiple load leaves.

The previous acceptance still applies. You can explicitly request a review

Dec 15 2017, 9:04 AM
niravd added inline comments to D41235: [DAGCOmbine] Ensure that (brcond (setcc ...)) is handled in a canonical manner..
Dec 15 2017, 8:56 AM
niravd created D41293: [DAG,X86] Improve Dependency analysis when doing multi-node Instruction Selection.
Dec 15 2017, 8:34 AM

Dec 13 2017

niravd accepted D41177: [DAGCombine] Move AND nodes to multiple load leaves.
Dec 13 2017, 6:26 AM
niravd added a comment to D41177: [DAGCombine] Move AND nodes to multiple load leaves.

So if the Load is the same size as the mask, we used insert an AND but we don't end up eliding it? That seems like a fence post error elsewhere and it may be worth checking that out.

Dec 13 2017, 6:26 AM

Dec 12 2017

niravd committed rL320505: [X86] Cleanup type conversion of 64-bit load-store pairs..
[X86] Cleanup type conversion of 64-bit load-store pairs.
Dec 12 2017, 10:26 AM
niravd closed D40918: [X86] Cleanup type conversion of 64-bit load-store pairs..
Dec 12 2017, 10:26 AM

Dec 11 2017

niravd updated the summary of D40969: [DAG] Elide overlapping stores.
Dec 11 2017, 8:19 AM
niravd updated the diff for D40918: [X86] Cleanup type conversion of 64-bit load-store pairs..

Rebase past nontemporal store update.

Dec 11 2017, 8:10 AM
niravd updated the diff for D40969: [DAG] Elide overlapping stores.

Rebase past updated nontemporal tests.

Dec 11 2017, 7:42 AM
niravd committed rL320379: [X86] Modify Nontemporal tests to avoid deadstore optimization..
[X86] Modify Nontemporal tests to avoid deadstore optimization.
Dec 11 2017, 7:36 AM

Dec 7 2017

niravd created D40969: [DAG] Elide overlapping stores.
Dec 7 2017, 10:05 AM
niravd added a comment to D40918: [X86] Cleanup type conversion of 64-bit load-store pairs..

Previously this transformation missed the the i64 load/non-temporal store pair and as a result they get split into a i32 operations. Because this aligns with the nontemporal store of the doubleword value in 12(%ebp) another transformation elides that pair (which given what this test look at is probably indicative that this test needs some sort of fencing added).

Dec 7 2017, 8:47 AM
niravd added a comment to D40918: [X86] Cleanup type conversion of 64-bit load-store pairs..

Are we supposed to be losing the non temporal property during this transformation

Dec 7 2017, 7:16 AM
niravd updated the diff for D40918: [X86] Cleanup type conversion of 64-bit load-store pairs..

Remove unused NewStore definitions.

Dec 7 2017, 7:14 AM

Dec 6 2017

niravd updated the diff for D40918: [X86] Cleanup type conversion of 64-bit load-store pairs..

Fix chain computation in 64-bit to 2x32-bit path.

Dec 6 2017, 1:10 PM
niravd planned changes to D40918: [X86] Cleanup type conversion of 64-bit load-store pairs..

This is missing a fix in the store generation. Fixing now.

Dec 6 2017, 1:04 PM
niravd created D40918: [X86] Cleanup type conversion of 64-bit load-store pairs..
Dec 6 2017, 1:01 PM
niravd added a comment to D40790: DAGCombiner bugfix in MergeStoresOfConstantsOrVecElts().

D40701 has landed so this should be set. Can you commit this test case?

Dec 6 2017, 7:56 AM
niravd committed rL319899: [ARM][AArch64][DAG] Reenable post-legalize store merge.
[ARM][AArch64][DAG] Reenable post-legalize store merge
Dec 6 2017, 7:31 AM
niravd closed D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge by committing rL319899: [ARM][AArch64][DAG] Reenable post-legalize store merge.
Dec 6 2017, 7:31 AM
niravd updated the diff for D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge.

AArch64 failing case is failing because we did not check that constucted BUILD_VECTOR elements were legal.

Dec 6 2017, 6:51 AM

Dec 5 2017

niravd added a comment to D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge.

I don't think I've seen that one. Can you send that test case to me so I can reproduce it?

Dec 5 2017, 1:37 PM
niravd accepted D39604: [DAGCombine] Move AND nodes to multiple load leaves.

Modulo the extra NodeToMask lines in zero extend case, this LGTM. Thanks!

Dec 5 2017, 6:04 AM
niravd added a comment to D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge.

Looking at LegalizeDAG, when we expand a TruncStore we do it as a store of a truncate which IIUC is strictly bitvector-level interpretation.

Yes, the LegalizeDAG code will likely crash on a float TruncStore (but they don't show up outside of x86, so probably nobody noticed). But the intent is pretty clear if you look at how the x86 backend lowers them. There's also a comment in SelectionDAGNodes.h. But anyway, it's obscure enough that it probably doesn't matter if you disable the optimization.

Dec 5 2017, 6:00 AM
niravd accepted D40833: [DAGCombine] isLegalNarrowLoad function (NFC).

LGTM.

Dec 5 2017, 5:49 AM

Dec 4 2017

niravd accepted D40444: [DAGCombine] Handle big endian correctly in CombineConsecutiveLoads.

LGTM. Thanks

Dec 4 2017, 10:48 AM
niravd requested review of D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge.
Dec 4 2017, 9:59 AM
niravd updated the diff for D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge.

I've disable store merges of FP constants with truncation as it's not clear what is the right behavior at the moment.

Dec 4 2017, 9:58 AM
niravd added inline comments to D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge.
Dec 4 2017, 9:56 AM
niravd added inline comments to D39604: [DAGCombine] Move AND nodes to multiple load leaves.
Dec 4 2017, 9:18 AM
niravd reopened D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge.
Dec 4 2017, 9:03 AM
niravd added inline comments to D39604: [DAGCombine] Move AND nodes to multiple load leaves.
Dec 4 2017, 8:33 AM

Dec 1 2017

niravd committed rL319607: [DAG][AArch64] Disable post-legalization store.
[DAG][AArch64] Disable post-legalization store
Dec 1 2017, 8:02 PM
niravd committed rL319587: [DAG][ARM] Revert "Reenable post-legalize store merge".
[DAG][ARM] Revert "Reenable post-legalize store merge"
Dec 1 2017, 1:56 PM
niravd committed rL319547: [ARM][DAG] Reenable post-legalize store merge.
[ARM][DAG] Reenable post-legalize store merge
Dec 1 2017, 6:50 AM
niravd closed D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge by committing rL319547: [ARM][DAG] Reenable post-legalize store merge.
Dec 1 2017, 6:50 AM
niravd accepted D40709: [ARM] Add and + load combine tests.

LGTM. Also you should generally feel free to just commit tests directly.

Dec 1 2017, 4:59 AM

Nov 30 2017

niravd created D40701: [ARM][AArch64][DAG] Reenable post-legalize store merge.
Nov 30 2017, 9:08 PM
niravd added a comment to D40444: [DAGCombine] Handle big endian correctly in CombineConsecutiveLoads.

Passing a struct of i16 by load to generate a BUILD_PAIR and

Nov 30 2017, 7:55 AM
niravd added inline comments to D39604: [DAGCombine] Move AND nodes to multiple load leaves.
Nov 30 2017, 7:32 AM

Nov 29 2017

niravd committed rL319331: [ARM][DAG] Revert Disable post-legalization store merge for ARM.
[ARM][DAG] Revert Disable post-legalization store merge for ARM
Nov 29 2017, 10:06 AM

Nov 27 2017

niravd added a comment to D39604: [DAGCombine] Move AND nodes to multiple load leaves.
Nov 27 2017, 10:53 AM
niravd added a comment to D40444: [DAGCombine] Handle big endian correctly in CombineConsecutiveLoads.

I've done a double check myself for all in-tree uses and all the uses of areNonVolatileConsecutiveLoads should be fine as well.

Nov 27 2017, 8:30 AM
niravd committed rL319036: [DAG] Do MergeConsecutiveStores again before Instruction Selection.
[DAG] Do MergeConsecutiveStores again before Instruction Selection
Nov 27 2017, 7:31 AM
niravd closed D33675: [DAG] Do MergeConsecutiveStores again before Instruction Selection by committing rL319036: [DAG] Do MergeConsecutiveStores again before Instruction Selection.
Nov 27 2017, 7:31 AM
niravd accepted D40193: [DAGCombine] Disable finding better chains for stores at O0.

LGTM.

Nov 27 2017, 7:27 AM

Nov 26 2017

niravd added a comment to D40444: [DAGCombine] Handle big endian correctly in CombineConsecutiveLoads.

This endianness problem is probably also latent where do load combination, but we should sink this check into areNonVolatileConsecutiveLoads and MatchLoadCombine.

Nov 26 2017, 10:57 AM

Nov 21 2017

niravd committed rL318797: Avoid unecessary opsize byte in segment move to memory.
Avoid unecessary opsize byte in segment move to memory
Nov 21 2017, 11:28 AM

Nov 20 2017

niravd committed rL318678: [X86] Avoid unecessary opsize byte in segment move to memory.
[X86] Avoid unecessary opsize byte in segment move to memory
Nov 20 2017, 10:39 AM
niravd closed D39847: [X86] Avoid unecessary opsize byte in segment move to memory by committing rL318678: [X86] Avoid unecessary opsize byte in segment move to memory.
Nov 20 2017, 10:39 AM

Nov 17 2017

niravd accepted D40193: [DAGCombine] Disable finding better chains for stores at O0.

LGTM modulo added check.

Nov 17 2017, 1:50 PM
niravd added a comment to D40193: [DAGCombine] Disable finding better chains for stores at O0.

We definitely shouldn't be doing this at O0. We should probably add the same check for FindBetterChain as well.

Nov 17 2017, 1:48 PM
niravd updated the diff for D39847: [X86] Avoid unecessary opsize byte in segment move to memory.

Add OpSizeIgnore modifier to allow consumption of optional 0x66 bytes and remove invalid instructions.

Nov 17 2017, 9:46 AM
niravd added a comment to D33675: [DAG] Do MergeConsecutiveStores again before Instruction Selection.

Ping. I think we're all set here. Can I get an LGTM?

Nov 17 2017, 6:29 AM

Nov 13 2017

niravd added a comment to D33675: [DAG] Do MergeConsecutiveStores again before Instruction Selection.

If the Aarch64 tests look good, I think this should be landable. The only remaining test that is more than a simple merge is the Mips/cconv/vector.ll test where the store merging allows the value stored on the stack to be forwarded to a now matching load and the stores and loads excised.

Nov 13 2017, 7:06 AM

Nov 9 2017

niravd created D39847: [X86] Avoid unecessary opsize byte in segment move to memory.
Nov 9 2017, 8:13 AM

Nov 6 2017

niravd updated the diff for D33675: [DAG] Do MergeConsecutiveStores again before Instruction Selection.

Resurrecting after long hiatus.

Nov 6 2017, 1:22 PM
niravd abandoned D18663: Cleanup Chain Handling in X86ISelLowering.

Already handled in D38547

Nov 6 2017, 7:13 AM

Oct 4 2017

niravd accepted D38547: [X86] Fix chains update when lowering BUILD_VECTOR to a vector load.

This is the same fix as in D18663 (which is now way out of date) but with a test case! This should be all of PR10114 and we can finally mark it fixed.

Oct 4 2017, 10:42 AM

Sep 18 2017

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,

Sep 18 2017, 1:26 PM

Sep 8 2017

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.

Sep 8 2017, 7:55 PM

Sep 5 2017

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.

Sep 5 2017, 6:29 AM