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 (252 w, 6 d)

Recent Activity

Tue, Aug 20

aditya_nandakumar committed rG08bd0808720d: [GlobalISel] Handle multiple registers in dbg.value intrinsic (authored by aditya_nandakumar).
[GlobalISel] Handle multiple registers in dbg.value intrinsic
Tue, Aug 20, 9:31 AM
aditya_nandakumar committed rL369403: [GlobalISel] Handle multiple registers in dbg.value intrinsic.
[GlobalISel] Handle multiple registers in dbg.value intrinsic
Tue, Aug 20, 9:30 AM
aditya_nandakumar added a comment to D66077: [GlobalISel] Handle multiple registers in dbg.value intrinsic.

I do not have submit permissions, please can someone submit on my behalf?

I can submit for you.

Tue, Aug 20, 9:30 AM · Restricted Project
aditya_nandakumar added a comment to D66077: [GlobalISel] Handle multiple registers in dbg.value intrinsic.

I do not have submit permissions, please can someone submit on my behalf?

Tue, Aug 20, 9:00 AM · Restricted Project

Thu, Aug 15

aditya_nandakumar accepted D66316: [GlobalISel] CSEMIRBuilder: Add support for G_GEP.
Thu, Aug 15, 3:36 PM · Restricted Project
aditya_nandakumar added inline comments to D66287: GlobalISel: add combiner for indexed loads and stores.
Thu, Aug 15, 2:37 PM · Restricted Project

Tue, Aug 13

aditya_nandakumar committed rGc65ac865c394: [GlobalISel]: Fix lowering of G_Shuffle_vector where we pick up the wrong… (authored by aditya_nandakumar).
[GlobalISel]: Fix lowering of G_Shuffle_vector where we pick up the wrong…
Tue, Aug 13, 6:24 PM
aditya_nandakumar committed rL368781: [GlobalISel]: Fix lowering of G_Shuffle_vector where we pick up the wrong….
[GlobalISel]: Fix lowering of G_Shuffle_vector where we pick up the wrong…
Tue, Aug 13, 6:24 PM
aditya_nandakumar closed D66182: [GlobalISel]: Fix lowering of G_Shuffle_vector where we pick up the wrong source index.

Committed in 368781.
I haven't had a chance to look into the DAG's implementation yet but our out of tree bots caught this.

Tue, Aug 13, 6:24 PM · Restricted Project
aditya_nandakumar created D66182: [GlobalISel]: Fix lowering of G_Shuffle_vector where we pick up the wrong source index.
Tue, Aug 13, 3:47 PM · Restricted Project
aditya_nandakumar committed rG615eee6402c8: [GlobalISel]: Fix lowering of G_SHUFFLE_VECTOR with scalar sources (authored by aditya_nandakumar).
[GlobalISel]: Fix lowering of G_SHUFFLE_VECTOR with scalar sources
Tue, Aug 13, 2:51 PM
aditya_nandakumar closed D66171: [GlobalISel]: Fix lowering of G_SHUFFLE_VECTOR with scalar sources.

368753.

Tue, Aug 13, 2:48 PM · Restricted Project
aditya_nandakumar committed rL368753: [GlobalISel]: Fix lowering of G_SHUFFLE_VECTOR with scalar sources.
[GlobalISel]: Fix lowering of G_SHUFFLE_VECTOR with scalar sources
Tue, Aug 13, 2:48 PM
aditya_nandakumar added a comment to D66171: [GlobalISel]: Fix lowering of G_SHUFFLE_VECTOR with scalar sources.

LGTM, although I rather dislike that we allow these. Maybe the IRTranslator should be trying to turn these into the equivalent G_BUILD_VECTOR?

Thanks for the quick review.
I agree that this is messy. Turning it into BUILD_VECTOR seems to be like an optimization for now - but I don't really have a better solution. It's probably worth creating a patch and seeing what others think about it.

Tue, Aug 13, 2:43 PM · Restricted Project
aditya_nandakumar created D66171: [GlobalISel]: Fix lowering of G_SHUFFLE_VECTOR with scalar sources.
Tue, Aug 13, 2:29 PM · Restricted Project
aditya_nandakumar accepted D65971: [GISel] Pass MachineRegisterInfo by const reference to matcher..
Tue, Aug 13, 1:42 PM · Restricted Project
aditya_nandakumar added a comment to D66109: GlobalISel: Change representation of shuffle masks.

Thanks for doing this. Overall the patch LGTM. On a side note, should we try to document all the links we have to IR in GlobalISel somewhere?

Not sure where that would go

Tue, Aug 13, 7:42 AM

Mon, Aug 12

aditya_nandakumar committed rG70fdfed45f04: [GlobalISel]: Add KnownBits for G_XOR (authored by aditya_nandakumar).
[GlobalISel]: Add KnownBits for G_XOR
Mon, Aug 12, 9:34 PM
aditya_nandakumar committed rL368648: [GlobalISel]: Add KnownBits for G_XOR.
[GlobalISel]: Add KnownBits for G_XOR
Mon, Aug 12, 9:32 PM
aditya_nandakumar closed D66119: [GlobalISel]: Add KnownBits for G_XOR.

368648

Mon, Aug 12, 9:32 PM · Restricted Project
aditya_nandakumar accepted D66118: GlobalISel: Add more verifier checks for G_SHUFFLE_VECTOR.
Mon, Aug 12, 4:36 PM
aditya_nandakumar added inline comments to D66111: GlobalISel: Implement lower for G_SHUFFLE_VECTOR.
Mon, Aug 12, 4:32 PM
aditya_nandakumar created D66119: [GlobalISel]: Add KnownBits for G_XOR.
Mon, Aug 12, 4:25 PM · Restricted Project
aditya_nandakumar accepted D66111: GlobalISel: Implement lower for G_SHUFFLE_VECTOR.

LGTM - conditional to the parent patch.

Mon, Aug 12, 4:02 PM
aditya_nandakumar added a comment to D66109: GlobalISel: Change representation of shuffle masks.

Thanks for doing this. Overall the patch LGTM. On a side note, should we try to document all the links we have to IR in GlobalISel somewhere?

Mon, Aug 12, 3:51 PM
aditya_nandakumar committed rG55371e697cd4: [GISel]: Fix a bug in KnownBits where we should have been using SizeInBits (authored by aditya_nandakumar).
[GISel]: Fix a bug in KnownBits where we should have been using SizeInBits
Mon, Aug 12, 2:28 PM
aditya_nandakumar committed rL368618: [GISel]: Fix a bug in KnownBits where we should have been using SizeInBits.
[GISel]: Fix a bug in KnownBits where we should have been using SizeInBits
Mon, Aug 12, 2:28 PM
aditya_nandakumar closed D66039: [GlobalISel]: Fix a bug in KnownBits where we should have been using SizeInBits.

368618.

Mon, Aug 12, 2:27 PM · Restricted Project

Fri, Aug 9

aditya_nandakumar created D66039: [GlobalISel]: Fix a bug in KnownBits where we should have been using SizeInBits.
Fri, Aug 9, 4:25 PM · Restricted Project

Wed, Aug 7

aditya_nandakumar committed rG277583ec0ab2: [GISel][NFC]: Make members of CombinerHelper accessible in derived classes (authored by aditya_nandakumar).
[GISel][NFC]: Make members of CombinerHelper accessible in derived classes
Wed, Aug 7, 7:27 PM
aditya_nandakumar committed rL368248: [GISel][NFC]: Make members of CombinerHelper accessible in derived classes.
[GISel][NFC]: Make members of CombinerHelper accessible in derived classes
Wed, Aug 7, 7:24 PM
aditya_nandakumar closed D65842: [GISel][NFC]: Make members of CombinerHelper accessible in derived classes.

Thanks. r368248

Wed, Aug 7, 7:24 PM · Restricted Project

Tue, Aug 6

aditya_nandakumar created D65842: [GISel][NFC]: Make members of CombinerHelper accessible in derived classes.
Tue, Aug 6, 6:36 PM · Restricted Project
aditya_nandakumar committed rG6bbfde5c48aa: [GISel]: Fix trivial build breakage (authored by aditya_nandakumar).
[GISel]: Fix trivial build breakage
Tue, Aug 6, 10:56 AM
aditya_nandakumar committed rL368067: [GISel]: Fix trivial build breakage.
[GISel]: Fix trivial build breakage
Tue, Aug 6, 10:56 AM
aditya_nandakumar committed rGc8ac029d0ae2: [GISel]: Add GISelKnownBits analysis (authored by aditya_nandakumar).
[GISel]: Add GISelKnownBits analysis
Tue, Aug 6, 10:20 AM
aditya_nandakumar committed rL368065: [GISel]: Add GISelKnownBits analysis.
[GISel]: Add GISelKnownBits analysis
Tue, Aug 6, 10:19 AM
aditya_nandakumar closed D65698: [GISel]: Add GISelKnownBits analysis .

Pushed in 368065.

Tue, Aug 6, 10:18 AM · Restricted Project

Mon, Aug 5

aditya_nandakumar updated the diff for D65698: [GISel]: Add GISelKnownBits analysis .

Updated.

Mon, Aug 5, 4:39 PM · Restricted Project
aditya_nandakumar added inline comments to D65698: [GISel]: Add GISelKnownBits analysis .
Mon, Aug 5, 1:01 PM · Restricted Project
aditya_nandakumar added inline comments to D65698: [GISel]: Add GISelKnownBits analysis .
Mon, Aug 5, 1:01 PM · Restricted Project

Sun, Aug 4

aditya_nandakumar added inline comments to D65698: [GISel]: Add GISelKnownBits analysis .
Sun, Aug 4, 5:19 PM · Restricted Project
aditya_nandakumar updated the diff for D65698: [GISel]: Add GISelKnownBits analysis .

Updated based on feedback.

Sun, Aug 4, 4:49 PM · Restricted Project
aditya_nandakumar added inline comments to D65698: [GISel]: Add GISelKnownBits analysis .
Sun, Aug 4, 4:48 PM · Restricted Project

Sat, Aug 3

aditya_nandakumar created D65698: [GISel]: Add GISelKnownBits analysis .
Sat, Aug 3, 9:28 AM · Restricted Project

Fri, Aug 2

aditya_nandakumar added a comment to D61321: [globalisel] Allow SrcOp to convert an APInt and render it as an immediate operand (MO.isImm() == true).

If option 2 is possible then go for it. If it's too much for work for now, then fallback to 1.

Fri, Aug 2, 12:51 PM · Restricted Project

Mon, Jul 29

aditya_nandakumar accepted D65415: GlobalISel: Replace artifact combiner checks with assert.
Mon, Jul 29, 10:31 PM

Jul 22 2019

aditya_nandakumar added a comment to D65048: [GISel]: Attach missing range metadata while translating G_LOADs.

LGTM. Are you planning on adding a MI computeKnownBits to use this? I will soon have a need for one and don't want to repeat work

Yes - that's how I stumbled on this.
We have an out of tree port of most of DAG's ComputeKnownBits and SimplifyDemandedBits already implemented. It hasn't been upstreamed yet because

  1. There was no need until now.
  2. Writing tests were/are pain and are dependent on combines which upstream didn't have a lot of.

    I'd be happy to create a patch for that assuming we decide to go down the route of a direct port of DAG like implementation.

I don't know what could be significantly different

@paquette is going to look at this from a design perspective but we haven't so far had a strong immediate need or the time to do this.

My thoughts are that computeKnownBits in SelectionDAG is a potentially very costly analysis, and we currently have magic recursion depth limits to prevent extremely long compile times. We don't have any evidence to support an alternative yet, but it would be nice to prototype different approaches for the GlobalISel equivalent. Maybe add caching support for the start, and/or implement it using a bottom up approach.

Jul 22 2019, 9:55 AM · Restricted Project

Jul 21 2019

aditya_nandakumar committed rGd7504a1569df: [GISel]: Attach missing range metadata while translating G_LOADs (authored by aditya_nandakumar).
[GISel]: Attach missing range metadata while translating G_LOADs
Jul 21 2019, 7:10 AM
aditya_nandakumar closed D65048: [GISel]: Attach missing range metadata while translating G_LOADs.

Thanks. r366656

Jul 21 2019, 7:10 AM · Restricted Project
aditya_nandakumar committed rL366656: [GISel]: Attach missing range metadata while translating G_LOADs.
[GISel]: Attach missing range metadata while translating G_LOADs
Jul 21 2019, 7:07 AM
aditya_nandakumar added a comment to D65048: [GISel]: Attach missing range metadata while translating G_LOADs.

LGTM. Are you planning on adding a MI computeKnownBits to use this? I will soon have a need for one and don't want to repeat work

Yes - that's how I stumbled on this.
We have an out of tree port of most of DAG's ComputeKnownBits and SimplifyDemandedBits already implemented. It hasn't been upstreamed yet because

  1. There was no need until now.
  2. Writing tests were/are pain and are dependent on combines which upstream didn't have a lot of.
Jul 21 2019, 6:53 AM · Restricted Project
aditya_nandakumar updated the diff for D65048: [GISel]: Attach missing range metadata while translating G_LOADs.

Attach range information only with one def

Jul 21 2019, 6:16 AM · Restricted Project
aditya_nandakumar added inline comments to D65048: [GISel]: Attach missing range metadata while translating G_LOADs.
Jul 21 2019, 6:16 AM · Restricted Project
aditya_nandakumar created D65048: [GISel]: Attach missing range metadata while translating G_LOADs.
Jul 21 2019, 5:08 AM · Restricted Project

Jul 18 2019

aditya_nandakumar abandoned D35594: [GISel]: ConstantFold operations when building MIR.

Yes - this is not required any more.

Jul 18 2019, 9:56 PM

Jul 8 2019

aditya_nandakumar added a comment to D64354: [AArch64][GlobalISel] Optimize conditional branches followed by unconditional branches.

Unless others have comments, this looks good to go in to me.

Jul 8 2019, 8:36 PM · Restricted Project
aditya_nandakumar added a comment to D64354: [AArch64][GlobalISel] Optimize conditional branches followed by unconditional branches.

I haven't read the patch yet, but just from reading the description I remembered that the DAG does this while building up the DAG. Maybe this needs to be done in the IRTranslator to keep this behavior? (This helps while comparing codegen with sdag/gisel).

Jul 8 2019, 9:56 AM · Restricted Project
aditya_nandakumar accepted D64348: GlobalISel: Convert some build functions to using SrcOp/DstOp.
Jul 8 2019, 8:53 AM
aditya_nandakumar accepted D64303: GlobalISel: Check address space when looking up iPTR size.

LGTM.

Jul 8 2019, 6:00 AM

Jul 6 2019

aditya_nandakumar accepted D64248: GlobalISel: widenScalar for G_BUILD_VECTOR.

LGTM.

Jul 6 2019, 3:10 AM

Jul 1 2019

aditya_nandakumar committed rG1023a2eca3f0: [GlobalISel]: Allow backends to custom legalize Intrinsics (authored by aditya_nandakumar).
[GlobalISel]: Allow backends to custom legalize Intrinsics
Jul 1 2019, 10:56 AM
aditya_nandakumar committed rL364821: [GlobalISel]: Allow backends to custom legalize Intrinsics.
[GlobalISel]: Allow backends to custom legalize Intrinsics
Jul 1 2019, 10:56 AM
aditya_nandakumar closed D31359: [GlobalISel]: Allow backends to custom legalize Intrinsics.

Thanks. Committed in 364821.

Jul 1 2019, 10:56 AM · Restricted Project
aditya_nandakumar added a comment to D31359: [GlobalISel]: Allow backends to custom legalize Intrinsics.

LGTM. I have a few patches depending on this now, and returning the bool is good enough. I think refining the result may be useful, but I haven't run into a concrete need for it yet. It would be more useful if we had a mechanism for detecting whether an intrinsic is legal or not, so that the selector verifier could error like any other instruction.

Jul 1 2019, 10:23 AM · Restricted Project

Jun 29 2019

aditya_nandakumar added inline comments to D31359: [GlobalISel]: Allow backends to custom legalize Intrinsics.
Jun 29 2019, 9:15 PM · Restricted Project

Jun 18 2019

aditya_nandakumar added a comment to D63496: [WIP] CodeGen: Prototype class for registers.

I did a quick glance and it seems reasonable.

Jun 18 2019, 10:14 AM

Jun 17 2019

aditya_nandakumar added a comment to D61321: [globalisel] Allow SrcOp to convert an APInt and render it as an immediate operand (MO.isImm() == true).

We could try to be heroic and use something stronger than unsigned for registers

Jun 17 2019, 1:36 PM · Restricted Project
aditya_nandakumar added a comment to D61321: [globalisel] Allow SrcOp to convert an APInt and render it as an immediate operand (MO.isImm() == true).

How terrible would it be to use APInt as the interface type, but then store it as int64_t?

That seems like a good compromise to me.

I don't think I agree. APInt as an interface type won't really save any typing, so what advantage would this compromise have? It's about the same to write SrcOp(APInt(x)) vs SrcOp(int64_t(x)), but the APInt version is more expensive and if we're not actually handling arbitrary precision it's quite a bit more confusing.

Jun 17 2019, 12:09 PM · Restricted Project
aditya_nandakumar added a comment to D61321: [globalisel] Allow SrcOp to convert an APInt and render it as an immediate operand (MO.isImm() == true).

How terrible would it be to use APInt as the interface type, but then store it as int64_t?

Jun 17 2019, 11:33 AM · Restricted Project
aditya_nandakumar accepted D63422: GlobalISel: Verify intrinsics.

LGTM.

Jun 17 2019, 7:10 AM

Jun 14 2019

aditya_nandakumar closed D63302: [GISel]: Fix broken matcher for hasOneUse.

363424

Jun 14 2019, 10:19 AM · Restricted Project
aditya_nandakumar committed rG5c7fcbdc4ba7: [GISel]: Fix pattern matcher for m_OneUse (authored by aditya_nandakumar).
[GISel]: Fix pattern matcher for m_OneUse
Jun 14 2019, 10:17 AM
aditya_nandakumar committed rL363424: [GISel]: Fix pattern matcher for m_OneUse.
[GISel]: Fix pattern matcher for m_OneUse
Jun 14 2019, 10:16 AM

Jun 13 2019

aditya_nandakumar created D63302: [GISel]: Fix broken matcher for hasOneUse.
Jun 13 2019, 3:24 PM · Restricted Project

May 16 2019

aditya_nandakumar accepted D62038: GlobalISel: Add fp<->int casts to MachineIRBuilder.
May 16 2019, 11:26 PM
aditya_nandakumar accepted D62037: GlobalISel: Add MIRBUilder wrappers for bitcount instructions.
May 16 2019, 11:25 PM

May 15 2019

aditya_nandakumar accepted D61980: GlobalISel: Add DstOp version of buildIntrinsic.

The patch looks good. One comment - does it make sense to have two functions or unify the implementation - make the non DstOp version forward to DstOp version?

It would require creating another vector constructing DstOps, which seems worse

May 15 2019, 8:45 PM
aditya_nandakumar accepted D61977: GlobalISel: Add G_FCOPYSIGN.
May 15 2019, 8:33 PM
aditya_nandakumar accepted D61978: GlobalISel: Add some FP instructions to MachineIRBuilder.
May 15 2019, 8:33 PM
aditya_nandakumar accepted D61979: GlobalISel: Add buildFConstant for APFloat.
May 15 2019, 8:30 PM
aditya_nandakumar accepted D61981: GlobalISel: Add buildXor/buildNot.
May 15 2019, 8:29 PM
aditya_nandakumar added a comment to D61980: GlobalISel: Add DstOp version of buildIntrinsic.

The patch looks good. One comment - does it make sense to have two functions or unify the implementation - make the non DstOp version forward to DstOp version?

May 15 2019, 8:28 PM

May 10 2019

aditya_nandakumar accepted D61813: [globalisel] Fix iterator invalidation in the extload combines.

Looks reasonable.

May 10 2019, 5:44 PM · Restricted Project

Apr 25 2019

aditya_nandakumar accepted D61157: [GlobalISel] Fix constrainOperandRegClass to insert copies in the right position for reg definitions.

LGTM. Thanks for fixing this.

Apr 25 2019, 7:21 PM · Restricted Project

Apr 17 2019

aditya_nandakumar committed rG92663376563d: [GISel]:IRTranslator: Prefer a buidInstr form that allows CSE of cast… (authored by aditya_nandakumar).
[GISel]:IRTranslator: Prefer a buidInstr form that allows CSE of cast…
Apr 17 2019, 7:18 PM
aditya_nandakumar closed D60844: [GISel]: IRTranslator: Prefer a buidInstr form that allows CSE of cast instructions.

r358637

Apr 17 2019, 7:18 PM · Restricted Project
aditya_nandakumar committed rL358637: [GISel]:IRTranslator: Prefer a buidInstr form that allows CSE of cast….
[GISel]:IRTranslator: Prefer a buidInstr form that allows CSE of cast…
Apr 17 2019, 7:17 PM
aditya_nandakumar created D60844: [GISel]: IRTranslator: Prefer a buidInstr form that allows CSE of cast instructions.
Apr 17 2019, 3:54 PM · Restricted Project

Apr 12 2019

aditya_nandakumar accepted D60580: [GlobalISel] Enable CSE in the IRTranslator & legalizer for -O0 with constants only.

LGTM. Thanks - please address the minor nit and proceed with two commits.

Apr 12 2019, 5:14 PM · Restricted Project
aditya_nandakumar added a comment to D60580: [GlobalISel] Enable CSE in the IRTranslator & legalizer for -O0 with constants only.

Looks reasonable. Could you please split out the CSEConfigBase into it's own commit?
Also I only see the includes for the CSEConfigBase.h but not the header file in this patch. Maybe you forgot to include it?

Apr 12 2019, 4:33 PM · Restricted Project

Apr 11 2019

aditya_nandakumar added inline comments to D60580: [GlobalISel] Enable CSE in the IRTranslator & legalizer for -O0 with constants only.
Apr 11 2019, 3:12 PM · Restricted Project
aditya_nandakumar added a comment to D60580: [GlobalISel] Enable CSE in the IRTranslator & legalizer for -O0 with constants only.

Looks mostly good - there are some minor comments from my side.

Apr 11 2019, 2:08 PM · Restricted Project
aditya_nandakumar accepted D60579: [AArch64][GlobalISel] Enable copy elision in the pre-legalizer combine and fix a crash.

Looks good to me.

Apr 11 2019, 1:55 PM · Restricted Project

Apr 3 2019

aditya_nandakumar accepted D60219: GlobalISel: Add another overload of buildUnmerge.

LGTM. Please add a test in MachineIRBuilderTest.cpp if possible.

Apr 3 2019, 10:54 AM

Mar 18 2019

aditya_nandakumar added a comment to D59444: [GlobalISel] Change MachineIRBuilder's SrcOp to contain subregister info.

In general I'm not too fond of building very target specific instructions involving subregs with the MachineIRBuilder (As these are only likely to be used in a few places in the selector). Specifically for this case of building a subreg, you can say

Builder.buildInstr(COPY, {Dst},{})
  .addReg(Src, 0, SubReg);

which is not too much larger than (and doesn't require making every SrcOp bigger).

Builder.buildInstr(COPY, {Dst}, {{Src, SubReg}});

While adding CSE support for COPYs with Subregs is great, I'm not sure how often it'll trigger and be useful (If it does occur then great). Also should we start adding support for building other kinds of target instructions - Adding immediate operands/target index nodes etc?
My approach for building non generic instructions have been with using the .addImm(..).addReg(...)... pattern - but if others strongly feel the need to add the ability to pass in all of the operands at once to the builder interfaces, and enabling CSE for them, then we can go ahead with this change.

Mar 18 2019, 10:48 AM · Restricted Project

Mar 13 2019

aditya_nandakumar added a comment to D58725: GlobalISel: Use multiple returns for intrinsic structs.

Looks reasonable to me but I'd like @volkan and @aditya_nandakumar to see how it impacts us.

Mar 13 2019, 1:56 PM

Mar 5 2019

aditya_nandakumar added a comment to D58419: [GISel]: Allow G_EXTRACT_VEC_ELT's result to be larger the source element type.

ping?

Mar 5 2019, 12:49 PM · Restricted Project

Feb 27 2019

aditya_nandakumar added a comment to D31359: [GlobalISel]: Allow backends to custom legalize Intrinsics.

I'm not sure I understand why this needs to be separate from legalizeCustom

Feb 27 2019, 5:32 PM · Restricted Project

Feb 26 2019

aditya_nandakumar updated the diff for D58419: [GISel]: Allow G_EXTRACT_VEC_ELT's result to be larger the source element type.

Missing braces.

Feb 26 2019, 5:42 PM · Restricted Project