Page MenuHomePhabricator

aditya_nandakumar (Aditya Nandakumar)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 17 2014, 4:45 PM (226 w, 2 d)

Recent Activity

Fri, Feb 15

aditya_nandakumar accepted D58261: GlobalISel: Fix inadequate verification of g_build_vector.
Fri, Feb 15, 6:59 AM
aditya_nandakumar accepted D58256: Try to organize MachineVerifier tests.

LGTM. Thanks for this change.

Fri, Feb 15, 6:58 AM

Thu, Feb 14

aditya_nandakumar committed rG0e362ec19a7e: [GISel][NFC]: Add methods to speed up insertion into GISelWorklist (authored by aditya_nandakumar).
[GISel][NFC]: Add methods to speed up insertion into GISelWorklist
Thu, Feb 14, 5:40 PM
aditya_nandakumar committed rL354093: [GISel][NFC]: Add methods to speed up insertion into GISelWorklist.
[GISel][NFC]: Add methods to speed up insertion into GISelWorklist
Thu, Feb 14, 5:40 PM
aditya_nandakumar closed D58073: [GlobalISel][NFC]: Add interface to reserve memory in GISelWorklist.

r354093

Thu, Feb 14, 5:40 PM · Restricted Project

Wed, Feb 13

aditya_nandakumar updated the diff for D58073: [GlobalISel][NFC]: Add interface to reserve memory in GISelWorklist.

Addressed some formatting related feedback.

Wed, Feb 13, 1:43 PM · Restricted Project
aditya_nandakumar added inline comments to D58073: [GlobalISel][NFC]: Add interface to reserve memory in GISelWorklist.
Wed, Feb 13, 1:43 PM · Restricted Project
aditya_nandakumar updated the diff for D58073: [GlobalISel][NFC]: Add interface to reserve memory in GISelWorklist.

Removed redundant NDEBUG checks.

Wed, Feb 13, 1:36 PM · Restricted Project
aditya_nandakumar updated the diff for D58073: [GlobalISel][NFC]: Add interface to reserve memory in GISelWorklist.

Changed the strategy to fix problems of DenseMap migration as well as silly traversals of MachineFunction just to estimate the size.

Wed, Feb 13, 1:09 PM · Restricted Project
aditya_nandakumar accepted D58150: [globalisel][combine] Split existing rules into a match and apply step.

Seems reasonable.

Wed, Feb 13, 6:45 AM · Restricted Project

Mon, Feb 11

aditya_nandakumar added inline comments to D58073: [GlobalISel][NFC]: Add interface to reserve memory in GISelWorklist.
Mon, Feb 11, 3:24 PM · Restricted Project
aditya_nandakumar added inline comments to D58073: [GlobalISel][NFC]: Add interface to reserve memory in GISelWorklist.
Mon, Feb 11, 2:15 PM · Restricted Project
aditya_nandakumar added inline comments to D58073: [GlobalISel][NFC]: Add interface to reserve memory in GISelWorklist.
Mon, Feb 11, 2:12 PM · Restricted Project
aditya_nandakumar created D58073: [GlobalISel][NFC]: Add interface to reserve memory in GISelWorklist.
Mon, Feb 11, 1:22 PM · Restricted Project

Sat, Feb 9

aditya_nandakumar accepted D57988: [GlobalISel] Regex the opcodes in unit test to fix non-deterministic ordering.

legalize-ext-csedebug-output.mir fails in a reverse iteration build because the OpcodeHitTable is a DenseMap and is iterated to print CSEInfo. We could have copied OpcodeHitTable to a vector and sorted it before iteration. Maybe that would be an overkill. I saw this comment in the test that we could regex the opcodes. Do we care about the order of the CSEInfo opcodes?

Sat, Feb 9, 8:44 AM · Restricted Project

Fri, Feb 8

aditya_nandakumar added inline comments to D57947: GlobalISel: Add G_FCANONICALIZE instruction.
Fri, Feb 8, 1:52 PM
aditya_nandakumar accepted D57947: GlobalISel: Add G_FCANONICALIZE instruction.

LGTM. Minor comment that's not really concerning - but if you feel like it please fix that as well.

Fri, Feb 8, 12:25 PM
aditya_nandakumar added inline comments to D57947: GlobalISel: Add G_FCANONICALIZE instruction.
Fri, Feb 8, 11:50 AM
aditya_nandakumar committed rG01e818a97d6a: [GISel][NFC]: Add missing call to record CSE hits in the CSEMIRBuilder (authored by aditya_nandakumar).
[GISel][NFC]: Add missing call to record CSE hits in the CSEMIRBuilder
Fri, Feb 8, 11:42 AM
aditya_nandakumar committed rL353553: [GISel][NFC]: Add missing call to record CSE hits in the CSEMIRBuilder.
[GISel][NFC]: Add missing call to record CSE hits in the CSEMIRBuilder
Fri, Feb 8, 11:42 AM
aditya_nandakumar closed D57932: [GISel][NFC]: Add missing call to record CSE hits in the CSEMIRBuilder.

Thanks. r353553

Fri, Feb 8, 11:41 AM · Restricted Project
aditya_nandakumar updated the diff for D57932: [GISel][NFC]: Add missing call to record CSE hits in the CSEMIRBuilder.

Added missing REQUIRES: asserts to the test.

Fri, Feb 8, 11:25 AM · Restricted Project
aditya_nandakumar updated the diff for D57932: [GISel][NFC]: Add missing call to record CSE hits in the CSEMIRBuilder.

Updated to add test case for CSE debug output and standardize the debug printing format inside CSE.

Fri, Feb 8, 11:14 AM · Restricted Project
aditya_nandakumar closed D57931: [GISel]: While constructing the GISelWorklist make sure we reserve at least the required size to the underlying dense map..
Fri, Feb 8, 10:03 AM · Restricted Project

Thu, Feb 7

aditya_nandakumar added a comment to D57931: [GISel]: While constructing the GISelWorklist make sure we reserve at least the required size to the underlying dense map..

Sorry - just realized that there was an LGTM, but was not accepted.

Thu, Feb 7, 7:35 PM · Restricted Project
aditya_nandakumar committed rGc7716756881e: [GISel]: While constructing the GISelWorklist make sure we reserve at least the… (authored by aditya_nandakumar).
[GISel]: While constructing the GISelWorklist make sure we reserve at least the…
Thu, Feb 7, 7:34 PM
aditya_nandakumar added a comment to D57931: [GISel]: While constructing the GISelWorklist make sure we reserve at least the required size to the underlying dense map..

r353498

Thu, Feb 7, 7:34 PM · Restricted Project
aditya_nandakumar committed rL353498: [GISel]: While constructing the GISelWorklist make sure we reserve at least the….
[GISel]: While constructing the GISelWorklist make sure we reserve at least the…
Thu, Feb 7, 7:32 PM
aditya_nandakumar added inline comments to D57931: [GISel]: While constructing the GISelWorklist make sure we reserve at least the required size to the underlying dense map..
Thu, Feb 7, 5:07 PM · Restricted Project
aditya_nandakumar created D57932: [GISel][NFC]: Add missing call to record CSE hits in the CSEMIRBuilder.
Thu, Feb 7, 4:55 PM · Restricted Project
aditya_nandakumar added inline comments to D57931: [GISel]: While constructing the GISelWorklist make sure we reserve at least the required size to the underlying dense map..
Thu, Feb 7, 4:55 PM · Restricted Project
aditya_nandakumar created D57931: [GISel]: While constructing the GISelWorklist make sure we reserve at least the required size to the underlying dense map..
Thu, Feb 7, 4:47 PM · Restricted Project

Wed, Feb 6

aditya_nandakumar accepted D57630: Move IR flag handling directly into builder calls for cases translated from Instructions in GlobalIsel.

LGTM. Thanks

Wed, Feb 6, 9:52 AM · Restricted Project

Tue, Feb 5

aditya_nandakumar committed rGfef7619b0553: [NFC][GlobalISel]: Add a convenience method to MachineInstrBuilder to simplify… (authored by aditya_nandakumar).
[NFC][GlobalISel]: Add a convenience method to MachineInstrBuilder to simplify…
Tue, Feb 5, 2:15 PM
aditya_nandakumar committed rL353223: [NFC][GlobalISel]: Add a convenience method to MachineInstrBuilder to simplify….
[NFC][GlobalISel]: Add a convenience method to MachineInstrBuilder to simplify…
Tue, Feb 5, 2:15 PM
aditya_nandakumar closed D57608: [NFC][GlobalISel]: Add a convenience method to MachineInstrBuilder to simplify getOperand(i).getReg().

r353223

Tue, Feb 5, 2:14 PM · Restricted Project
aditya_nandakumar accepted D57656: GlobalISel: Fix artifact combiner constant legality checks for vectors.

Thanks. LGTM.

Tue, Feb 5, 7:55 AM

Mon, Feb 4

aditya_nandakumar added inline comments to D57656: GlobalISel: Fix artifact combiner constant legality checks for vectors.
Mon, Feb 4, 6:17 PM
aditya_nandakumar committed rG9b6b9a5791db: [Tablegen][DAG]: Fix build breakage when LLVM_ENABLE_DAGISEL_COV=1 (authored by aditya_nandakumar).
[Tablegen][DAG]: Fix build breakage when LLVM_ENABLE_DAGISEL_COV=1
Mon, Feb 4, 1:08 PM
aditya_nandakumar committed rL353091: [Tablegen][DAG]: Fix build breakage when LLVM_ENABLE_DAGISEL_COV=1.
[Tablegen][DAG]: Fix build breakage when LLVM_ENABLE_DAGISEL_COV=1
Mon, Feb 4, 1:06 PM
aditya_nandakumar updated the diff for D57608: [NFC][GlobalISel]: Add a convenience method to MachineInstrBuilder to simplify getOperand(i).getReg().

Updated to use shorter name.

Mon, Feb 4, 11:27 AM · Restricted Project
aditya_nandakumar accepted D57650: GlobalISel: Enforce operand types for constants.

Looks great. Thanks for fixing this..

Mon, Feb 4, 7:14 AM
aditya_nandakumar accepted D57651: GlobalISel: Fix CSE handling of buildConstant.

LGTM. Please move the EXPECT_EQ changes to the other commit.

Mon, Feb 4, 7:11 AM
aditya_nandakumar added a comment to D57652: GlobalISel: Improve gtest usage.

Please include the changes to CSETest.cpp in https://reviews.llvm.org/D57651 here instead and commit. Thanks

Mon, Feb 4, 7:09 AM
aditya_nandakumar accepted D57652: GlobalISel: Improve gtest usage.
Mon, Feb 4, 6:56 AM
aditya_nandakumar accepted D57684: GlobalISel: Allow constructing SrcOp/DstOp from MachineOperand.

Please add a test case. LGTM.

Mon, Feb 4, 6:44 AM

Fri, Feb 1

aditya_nandakumar added a comment to D57608: [NFC][GlobalISel]: Add a convenience method to MachineInstrBuilder to simplify getOperand(i).getReg().

If we're going to add a helper to shave some characters off, we might as well go for something really short like getOpReg().

Fri, Feb 1, 4:10 PM · Restricted Project
aditya_nandakumar added inline comments to D57608: [NFC][GlobalISel]: Add a convenience method to MachineInstrBuilder to simplify getOperand(i).getReg().
Fri, Feb 1, 1:29 PM · Restricted Project
aditya_nandakumar created D57608: [NFC][GlobalISel]: Add a convenience method to MachineInstrBuilder to simplify getOperand(i).getReg().
Fri, Feb 1, 11:56 AM · Restricted Project

Tue, Jan 29

aditya_nandakumar abandoned D52131: [GISel][NFC]: Make MachineIRBuilder fully stateless.
Tue, Jan 29, 9:21 AM

Thu, Jan 24

aditya_nandakumar committed rL352126: [GISel]: Change how CSE is enabled by default for each pass.
[GISel]: Change how CSE is enabled by default for each pass
Thu, Jan 24, 3:11 PM
aditya_nandakumar closed D57178: [GISel]: Change how CSE is enabled by default for each pass.

Thanks. Submitted in r352126.

Thu, Jan 24, 3:11 PM
aditya_nandakumar updated the diff for D57178: [GISel]: Change how CSE is enabled by default for each pass.

Updated based on feedback.

Thu, Jan 24, 1:37 PM
aditya_nandakumar created D57178: [GISel]: Change how CSE is enabled by default for each pass.
Thu, Jan 24, 11:58 AM

Tue, Jan 22

aditya_nandakumar accepted D56835: GlobalISel: Make buildConstant handle vectors.

Thanks. Looks good.

Tue, Jan 22, 1:08 PM
aditya_nandakumar added a comment to D56835: GlobalISel: Make buildConstant handle vectors.

Could you please add a test case for the vector build constant case? I suspect unless there is already a target to use this, unit test might be the only way.

Tue, Jan 22, 8:00 AM
aditya_nandakumar accepted D56833: GlobalISel: Disallow vectors for G_CONSTANT/G_FCONSTANT.
Tue, Jan 22, 7:39 AM

Jan 18 2019

aditya_nandakumar accepted D56832: GlobalISel: Verify G_ICMP/G_FCMP vector types.
Jan 18 2019, 7:53 AM
aditya_nandakumar accepted D56893: GlobalISel: Fix out of bounds crashes in verifier.
Jan 18 2019, 7:49 AM
aditya_nandakumar accepted D56894: GlobalISel: Verify G_BITCAST.
Jan 18 2019, 7:35 AM

Jan 15 2019

aditya_nandakumar closed D52803: [GISel]: Add support for CSEing continuously during GISel passes.

Submitted in r351283.

Jan 15 2019, 4:46 PM
aditya_nandakumar committed rL351283: [GISel]: Add support for CSEing continuously during GISel passes..
[GISel]: Add support for CSEing continuously during GISel passes.
Jan 15 2019, 4:44 PM
aditya_nandakumar updated the diff for D52803: [GISel]: Add support for CSEing continuously during GISel passes.

Rebased, updated based on feedback.
With this change, now they're disabled by default (for O0).

Jan 15 2019, 4:08 PM
aditya_nandakumar added a comment to D56706: GlobalISel: Combine trunc with constant.

It depends on it only really for the tests. It shouldn’t be needed for correctness

Jan 15 2019, 3:40 PM
aditya_nandakumar added a comment to D56706: GlobalISel: Combine trunc with constant.

Is this required for correctness? This looks like something that would fall in an actual combiner (vs the artifact combiner).

Jan 15 2019, 12:43 PM

Jan 14 2019

aditya_nandakumar updated the diff for D52803: [GISel]: Add support for CSEing continuously during GISel passes.

Updated to add a CSEConfig class that describes what instructions should be CSEd.
Additionally, updated IRTranslator and Legalizer to only perform CSE for G_CONSTANTS for -O0.

Jan 14 2019, 1:47 PM

Jan 9 2019

aditya_nandakumar added a comment to D56307: GlobalISel: Remove invalid assert.

While splatting is convenient (compared with build_vector of constants), I realized from discussion in https://reviews.llvm.org/D56379, this would make it inconsistent with rest of GISel.

Jan 9 2019, 9:41 AM
aditya_nandakumar added a comment to D56379: GlobalISel: Implement fewerElementsVector for constants.

See also D56307

Jan 9 2019, 9:36 AM
aditya_nandakumar added a comment to D56379: GlobalISel: Implement fewerElementsVector for constants.

This is strange. G_CONSTANT isn't supposed to be able to splat the constant into a vector, so this is an oversight in the MachineVerifier for not catching this case.

Constant vectors are handled in the IRTranslator as G_BUILD_VECTORs of the constant element values. Sorry about the confusion.

They aren't handled this way in the MachineIRBuilder. I would prefer it if G_CONSTANT did splat, since I always found this extremely annoying in SelectionDAG

Jan 9 2019, 9:34 AM
aditya_nandakumar accepted D56477: GlobalISel: Use appropriate extension for legalizing select conditions.

LGTM with test case.

Jan 9 2019, 9:10 AM
aditya_nandakumar added a comment to D56477: GlobalISel: Use appropriate extension for legalizing select conditions.

Hi Matt, this change looks good to me. Please add a test case and commit.

Jan 9 2019, 9:05 AM
aditya_nandakumar accepted D56478: GlobalISel: Add comment to clarify G_BUILD_VECTOR.

Looks good. Thanks.

Jan 9 2019, 8:38 AM

Jan 8 2019

aditya_nandakumar accepted D56307: GlobalISel: Remove invalid assert.

LGTM.

Jan 8 2019, 9:25 AM
aditya_nandakumar accepted D56352: GlobalISel: Implement widenScalar for implicit_def.

Looks good. Thanks

Jan 8 2019, 9:15 AM
aditya_nandakumar added a comment to D52803: [GISel]: Add support for CSEing continuously during GISel passes.

By default the CSE is enabled during IRTranslator and Legalizer now (disable with -enable-cse-in-legalizer/irtranslator=0).

I naively wonder if unconditionally enabling a CSE optimization even at -O0 is going to lead to a poorer debug experience in some cases?

Yes this is definitely an important question. I only enabled them by default because I wanted to test that CSE works with the pipeline. I can definitely disable CSE for O0 for each of the passes and add some more tests (besides making the unit test more comprehensive) that make sure CSE works in pipeline.

I've also updated most tests (except some tests where it's not obvious what the test is doing where CSE is disabled).

If CSE should be enabled by default, I guess that we'll need to figure out what the tests are supposed to be doing so that there aren't many tests mainly testing non-default behaviour?

I updated most of the tests that just run a single GISel pass. I was hesitant to update the tests that take it all the way to assembly (that were not auto generated).

Jan 8 2019, 7:57 AM

Jan 7 2019

aditya_nandakumar updated the diff for D52803: [GISel]: Add support for CSEing continuously during GISel passes.

Rebased, and updated to the new MachineIRBuilder interface.
By default the CSE is enabled during IRTranslator and Legalizer now (disable with -enable-cse-in-legalizer/irtranslator=0).
Added a helper class called GISelObserverWrapper which provides a single observer that can call several observers which can be used to register as the delegate to MachineFunction.
I've also updated most tests (except some tests where it's not obvious what the test is doing where CSE is disabled).

Jan 7 2019, 3:40 PM

Dec 17 2018

aditya_nandakumar added a comment to D55668: Add FMF management to common fp intrinsics in GlobalIsel.

Minor nit about using -> on MachineInstr builders directly.

Dec 17 2018, 8:07 PM

Dec 13 2018

aditya_nandakumar accepted D52947: [globalisel][combiner] Make the CombinerChangeObserver a MachineFunction::Delegate.

Thanks Daniel. LGTM with changes to the nitpicks..

Dec 13 2018, 1:11 PM
aditya_nandakumar added inline comments to D52947: [globalisel][combiner] Make the CombinerChangeObserver a MachineFunction::Delegate.
Dec 13 2018, 12:01 PM

Dec 12 2018

aditya_nandakumar accepted D55623: [globalisel] Add GISelChangeObserver::changingInstr().

LGTM. Thanks for this.

Dec 12 2018, 2:36 PM
aditya_nandakumar accepted D55611: [globalisel] Rename GISelChangeObserver's erasedInstr() to erasingInstr() and related nits. NFC.

Thanks. LGTM.

Dec 12 2018, 12:56 PM

Dec 11 2018

aditya_nandakumar closed D55516: [GISel]: Add MachineIRBuilder support for passing in Flags while building instructions.

Submitted in r348893. Thanks for reviewing.

Dec 11 2018, 12:07 PM
aditya_nandakumar committed rL348893: [GISel]: Add MachineIRBuilder support for passing in Flags while building.
[GISel]: Add MachineIRBuilder support for passing in Flags while building
Dec 11 2018, 12:07 PM

Dec 10 2018

aditya_nandakumar closed D55294: [GISel]: Refactor MachineIRBuilder to allow passing additional parameters to build Instrs.

Pushed in r348815

Dec 10 2018, 4:52 PM
aditya_nandakumar committed rL348815: [GISel]: Refactor MachineIRBuilder to allow passing additional parameters to….
[GISel]: Refactor MachineIRBuilder to allow passing additional parameters to…
Dec 10 2018, 4:52 PM
aditya_nandakumar added a comment to D55294: [GISel]: Refactor MachineIRBuilder to allow passing additional parameters to build Instrs.


Added a clang-tidy patch which should automatically update most API calls to the new API. When possible (most trivial cases), it adds {} around sources and destinations.

Dec 10 2018, 3:01 PM
aditya_nandakumar updated the diff for D55294: [GISel]: Refactor MachineIRBuilder to allow passing additional parameters to build Instrs.

Updated based on Justin/Volkan's feedback. Moved buildI/FCmp as well to buildInstr.

Dec 10 2018, 1:38 PM
aditya_nandakumar added a comment to D55294: [GISel]: Refactor MachineIRBuilder to allow passing additional parameters to build Instrs.

All the buildInstr support so far has been for instructions that have register only operands. The ones missing are probably non register operands (such as predicates, immediate, FPConstant etc)

Dec 10 2018, 12:48 PM
aditya_nandakumar added inline comments to D55516: [GISel]: Add MachineIRBuilder support for passing in Flags while building instructions.
Dec 10 2018, 11:04 AM
aditya_nandakumar added a parent revision for D55516: [GISel]: Add MachineIRBuilder support for passing in Flags while building instructions: D55294: [GISel]: Refactor MachineIRBuilder to allow passing additional parameters to build Instrs.
Dec 10 2018, 9:45 AM
aditya_nandakumar created D55516: [GISel]: Add MachineIRBuilder support for passing in Flags while building instructions.
Dec 10 2018, 8:57 AM

Dec 7 2018

aditya_nandakumar updated the diff for D55294: [GISel]: Refactor MachineIRBuilder to allow passing additional parameters to build Instrs.

Added support for a few other instructions such as Unmerge, Merge, UADDE etc.
Now all instructions which only involve register operands go through buildInstr. So calling buildInstr(ADD, s32, {a, b}) should be equivalent to buildAdd(s32, {a, b});

Dec 7 2018, 3:24 PM
aditya_nandakumar added a comment to D55392: [GlobalISel] Add IR translation support for the @llvm.log10 intrinsic.

Thanks. This patch looks good. Would be nice if there's test to make sure the G_FLOG10 is being selected as well.

Dec 7 2018, 10:59 AM

Dec 5 2018

aditya_nandakumar updated the diff for D55294: [GISel]: Refactor MachineIRBuilder to allow passing additional parameters to build Instrs.

Rebased.
Also removed some temporary code that snuck in.

Dec 5 2018, 2:01 PM
aditya_nandakumar closed D54980: [NFC][GISel]: Provide an standard interface to observe changes across GISel passes.

Thanks Volkan. Committed in r348406.

Dec 5 2018, 12:18 PM
aditya_nandakumar committed rL348406: [GISel]: Provide standard interface to observe changes in GISel passes.
[GISel]: Provide standard interface to observe changes in GISel passes
Dec 5 2018, 12:18 PM

Dec 4 2018

aditya_nandakumar added a comment to D55294: [GISel]: Refactor MachineIRBuilder to allow passing additional parameters to build Instrs.

I also added{F7649492}

Dec 4 2018, 1:26 PM
aditya_nandakumar created D55294: [GISel]: Refactor MachineIRBuilder to allow passing additional parameters to build Instrs.
Dec 4 2018, 1:20 PM

Nov 27 2018

aditya_nandakumar created D54980: [NFC][GISel]: Provide an standard interface to observe changes across GISel passes.
Nov 27 2018, 3:22 PM

Nov 6 2018

aditya_nandakumar added a comment to D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.

This looks good to me assuming we go with the extra opcode..

Nov 6 2018, 12:17 PM