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 (260 w, 5 d)

Recent Activity

Mon, Oct 7

aditya_nandakumar accepted D68582: GlobalISel: Add target pre-isel instructions.
Mon, Oct 7, 10:19 AM
aditya_nandakumar added a comment to D68582: GlobalISel: Add target pre-isel instructions.

I really like the approach here. Thanks for working on this.
LGTM.

Mon, Oct 7, 8:49 AM

Sat, Oct 5

aditya_nandakumar accepted D68532: GlobalISel: Partially implement lower for G_EXTRACT.
Sat, Oct 5, 3:31 PM

Mon, Sep 30

aditya_nandakumar accepted D68243: GlobalISel: Implement widenScalar for G_SITOFP/G_UITOFP sources.
Mon, Sep 30, 2:05 PM

Mon, Sep 23

aditya_nandakumar committed rG72a4621cdf0b: [TableGen] Emit OperandType enums for RegisterOperands/RegisterClasses (authored by aditya_nandakumar).
[TableGen] Emit OperandType enums for RegisterOperands/RegisterClasses
Mon, Sep 23, 11:52 AM
aditya_nandakumar committed rL372641: [TableGen] Emit OperandType enums for RegisterOperands/RegisterClasses.
[TableGen] Emit OperandType enums for RegisterOperands/RegisterClasses
Mon, Sep 23, 11:51 AM
aditya_nandakumar closed D66773: [TableGen] Emit OperandType enums for RegisterOperands/RegisterClasses.

Committed revision 372641

Mon, Sep 23, 11:51 AM · Restricted Project
aditya_nandakumar added a comment to D67133: [GlobalISel]: Fix some non determinism exposed in CSE due to not notifying observers about mutations + add verification for CSE.

Ping. Can we combine the delegate and observer installers?

Mon, Sep 23, 11:42 AM · Restricted Project

Sep 11 2019

aditya_nandakumar accepted D67279: GlobalISel/AMDGPU: Legalize G_FMAD.
Sep 11 2019, 1:17 PM
aditya_nandakumar added inline comments to D67279: GlobalISel/AMDGPU: Legalize G_FMAD.
Sep 11 2019, 12:53 PM

Sep 9 2019

aditya_nandakumar accepted D67380: [globalisel][knownbits] Allow targets to call GISelKnownBits::computeKnownBitsImpl().
Sep 9 2019, 4:39 PM · Restricted Project
aditya_nandakumar committed rG5112b71126a1: [GlobalISel]: Fix a bug where we could dereference None (authored by aditya_nandakumar).
[GlobalISel]: Fix a bug where we could dereference None
Sep 9 2019, 3:53 PM
aditya_nandakumar committed rL371465: [GlobalISel]: Fix a bug where we could dereference None.
[GlobalISel]: Fix a bug where we could dereference None
Sep 9 2019, 3:50 PM

Sep 6 2019

aditya_nandakumar added a comment to D67275: GlobalISel: Add G_FMAD instruction.

Looks good. Is this something that's only produced during legalization of targets?

It's produced by optimization combines in the DAG mostly, but AMDGPU does use this in a couple custom legalizations

If this is used only by one target (AMDGPU) (and does not require further legalizations), one alternative could be making this a target pseudo.

It's already a generic node for the DAG. ARM could use it, but doesn't. In practice it's a huge pain to make this target specific, since it shares most combines with FMA.

Makes sense.

We also do have a problem now where we don't really have an equivalent of target specific nodes in GlobalISel. Intrinsics are close, but don't quite fill the purpose and would litter the IR intrinsic namespace

Sep 6 2019, 8:45 AM
aditya_nandakumar added a comment to D67275: GlobalISel: Add G_FMAD instruction.

Looks good. Is this something that's only produced during legalization of targets?

It's produced by optimization combines in the DAG mostly, but AMDGPU does use this in a couple custom legalizations

Sep 6 2019, 7:52 AM
aditya_nandakumar accepted D67275: GlobalISel: Add G_FMAD instruction.

Looks good. Is this something that's only produced during legalization of targets?

Sep 6 2019, 7:35 AM

Sep 4 2019

aditya_nandakumar added inline comments to D67133: [GlobalISel]: Fix some non determinism exposed in CSE due to not notifying observers about mutations + add verification for CSE.
Sep 4 2019, 11:53 AM · Restricted Project

Sep 3 2019

aditya_nandakumar accepted D67131: [globalisel] Support trivial COPY in GISelKnownBits.
Sep 3 2019, 3:48 PM · Restricted Project
aditya_nandakumar created D67133: [GlobalISel]: Fix some non determinism exposed in CSE due to not notifying observers about mutations + add verification for CSE.
Sep 3 2019, 3:44 PM · Restricted Project

Aug 30 2019

aditya_nandakumar committed rL370557: Request commit access for aditya_nandakumar.
Request commit access for aditya_nandakumar
Aug 30 2019, 5:24 PM

Aug 29 2019

aditya_nandakumar accepted D66956: GlobalISel: Don't compute known bits for non-integral GEP.
Aug 29 2019, 10:32 AM

Aug 28 2019

aditya_nandakumar accepted D66921: GlobalISel: Add known bits to InstructionSelector.
Aug 28 2019, 8:54 PM
aditya_nandakumar added a comment to D66921: GlobalISel: Add known bits to InstructionSelector.

Hi Matt, this looks reasonable - though when (if) we add a more complex implementation (perhaps precomputes knownbits) we might have to revisit this.
For now, it's close to zero cost. LGTM.

Aug 28 2019, 8:54 PM
aditya_nandakumar accepted D66923: GlobalISel: Add maskedValueIsZero and signBitIsZero to known bits.
Aug 28 2019, 8:51 PM

Aug 20 2019

aditya_nandakumar committed rG08bd0808720d: [GlobalISel] Handle multiple registers in dbg.value intrinsic (authored by aditya_nandakumar).
[GlobalISel] Handle multiple registers in dbg.value intrinsic
Aug 20 2019, 9:31 AM
aditya_nandakumar committed rL369403: [GlobalISel] Handle multiple registers in dbg.value intrinsic.
[GlobalISel] Handle multiple registers in dbg.value intrinsic
Aug 20 2019, 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.

Aug 20 2019, 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?

Aug 20 2019, 9:00 AM · Restricted Project

Aug 15 2019

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

Aug 13 2019

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…
Aug 13 2019, 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…
Aug 13 2019, 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.

Aug 13 2019, 6:24 PM · Restricted Project
aditya_nandakumar created D66182: [GlobalISel]: Fix lowering of G_Shuffle_vector where we pick up the wrong source index.
Aug 13 2019, 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
Aug 13 2019, 2:51 PM
aditya_nandakumar closed D66171: [GlobalISel]: Fix lowering of G_SHUFFLE_VECTOR with scalar sources.

368753.

Aug 13 2019, 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
Aug 13 2019, 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.

Aug 13 2019, 2:43 PM · Restricted Project
aditya_nandakumar created D66171: [GlobalISel]: Fix lowering of G_SHUFFLE_VECTOR with scalar sources.
Aug 13 2019, 2:29 PM · Restricted Project
aditya_nandakumar accepted D65971: [GISel] Pass MachineRegisterInfo by const reference to matcher..
Aug 13 2019, 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

Aug 13 2019, 7:42 AM

Aug 12 2019

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

368648

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

LGTM - conditional to the parent patch.

Aug 12 2019, 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?

Aug 12 2019, 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
Aug 12 2019, 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
Aug 12 2019, 2:28 PM
aditya_nandakumar closed D66039: [GlobalISel]: Fix a bug in KnownBits where we should have been using SizeInBits.

368618.

Aug 12 2019, 2:27 PM · Restricted Project

Aug 9 2019

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

Aug 7 2019

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
Aug 7 2019, 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
Aug 7 2019, 7:24 PM
aditya_nandakumar closed D65842: [GISel][NFC]: Make members of CombinerHelper accessible in derived classes.

Thanks. r368248

Aug 7 2019, 7:24 PM · Restricted Project

Aug 6 2019

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

Pushed in 368065.

Aug 6 2019, 10:18 AM · Restricted Project

Aug 5 2019

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

Updated.

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

Aug 4 2019

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

Updated based on feedback.

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

Aug 3 2019

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

Aug 2 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).

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

Aug 2 2019, 12:51 PM · Restricted Project

Jul 29 2019

aditya_nandakumar accepted D65415: GlobalISel: Replace artifact combiner checks with assert.
Jul 29 2019, 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