qcolombet (Quentin Colombet)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 17 2012, 10:03 AM (257 w, 3 d)

Recent Activity

Fri, Nov 17

qcolombet committed rL318589: [AArch64] Map G_LOAD on FPR when the definition goes to a copy to FPR.
[AArch64] Map G_LOAD on FPR when the definition goes to a copy to FPR
Fri, Nov 17, 8:29 PM
qcolombet committed rL318588: [AArch64] Map G_STORE on FPR when the source comes from a FPR copy.
[AArch64] Map G_STORE on FPR when the source comes from a FPR copy
Fri, Nov 17, 8:29 PM
qcolombet committed rL318587: [RegisterBankInfo] Relax the assert of having matching type sizes on default….
[RegisterBankInfo] Relax the assert of having matching type sizes on default…
Fri, Nov 17, 8:29 PM
qcolombet committed rL318586: [AArch64][RegisterBankInfo] Teach instruction mapping about gpr32 -> fpr16….
[AArch64][RegisterBankInfo] Teach instruction mapping about gpr32 -> fpr16…
Fri, Nov 17, 8:29 PM
qcolombet accepted D38128: Handle COPYs of physregs better (regalloc hints).

Still LGTM :).
It makes sense to have it behind a hook for now, but could you follow-up with the targets owners so that it gets to be the default and we can get rid of the hook.
I believe this is general goodness and that we should use it for all targets going forward.

Fri, Nov 17, 10:17 AM

Wed, Nov 15

qcolombet accepted D39742: Add backend name to Target to enable runtime info to be fed back into TableGen.
Wed, Nov 15, 2:59 PM

Mon, Nov 13

qcolombet accepted D39986: [LSR] Expand: Use the replaced value's debug loc (PR25630).

LGTM. Nitpicks below.

Mon, Nov 13, 4:39 PM

Thu, Nov 2

qcolombet committed rL317287: [AArch64][RegisterBankInfo] Add mapping for G_FPEXT..
[AArch64][RegisterBankInfo] Add mapping for G_FPEXT.
Thu, Nov 2, 4:38 PM
qcolombet committed rL317286: [AArch64][RegisterBankInfo] Add FPR16 support in value mapping..
[AArch64][RegisterBankInfo] Add FPR16 support in value mapping.
Thu, Nov 2, 4:38 PM
qcolombet added inline comments to D38611: Fix X86 regression on linpack .
Thu, Nov 2, 12:41 PM
qcolombet added a comment to D36504: [CodeGenPrepare][WIP] Convert uncond. branch to return into a return to help with shrink-wrapping.

I don't know enough about shrink wrapping to know if that is feasible or not. It doesn't seem practical to me, but maybe Francis or Quentin can comment?

We'd actually want to do something relate to solve https://bugs.llvm.org/show_bug.cgi?id=33868.
We didn't have a chance to look at it yet.

Thu, Nov 2, 10:50 AM

Oct 24 2017

qcolombet updated the diff for D39034: [WIP][GlobalISel][TableGen] Optimize MatchTable for faster instruction selection.

Move back the sorting of the Rules in main function.
This is not part of the optimization process.

Oct 24 2017, 4:25 PM
qcolombet added a comment to D39034: [WIP][GlobalISel][TableGen] Optimize MatchTable for faster instruction selection.

One thing to note: When you try a group, there is no way out. I.e., the assumption is that groups are mutually independent. This is actually why I haven't push for more nesting level, because the patch doesn't provide a way to check that this is true.

Oct 24 2017, 3:21 PM
qcolombet added a comment to D39034: [WIP][GlobalISel][TableGen] Optimize MatchTable for faster instruction selection.

For this, I was thinking there would be two kinds of partitioner. Those that deal with structure (number of operands, opcodes?*, nested instructions, etc.) and those that deal with predicates.

Oct 24 2017, 3:17 PM
qcolombet added a comment to D39034: [WIP][GlobalISel][TableGen] Optimize MatchTable for faster instruction selection.

Ping?

Oct 24 2017, 10:25 AM

Oct 23 2017

qcolombet accepted D37065: Ensure standard pseudo instructions (TargetOpcode::*) are compatible with guessInstructionProperties=0.

Sounds good to me.

Oct 23 2017, 3:34 PM
qcolombet added a comment to D37065: Ensure standard pseudo instructions (TargetOpcode::*) are compatible with guessInstructionProperties=0.

Hi Alex,

Oct 23 2017, 11:44 AM

Oct 17 2017

qcolombet added a comment to D39034: [WIP][GlobalISel][TableGen] Optimize MatchTable for faster instruction selection.

For the record, what the grouping does is pretty much flattening the predicate class hierarchy. Long term, we may want to have a simpler class hierarchy.
Also, I have tried to same on memory comsumption and speed of tablegen, in particular, we duplicate operand indices and instruction identifier all over the place to make the checking of predicate correct. (isIdentical)

Oct 17 2017, 7:51 PM
qcolombet created D39034: [WIP][GlobalISel][TableGen] Optimize MatchTable for faster instruction selection.
Oct 17 2017, 7:47 PM

Oct 16 2017

qcolombet committed rL315947: Re-apply [AArch64][RegisterBankInfo] Use the statically computed mappings for….
Re-apply [AArch64][RegisterBankInfo] Use the statically computed mappings for…
Oct 16 2017, 3:28 PM
qcolombet committed rL315946: [AArch64][RegisterBankInfo] Add mapping support for G_BITCAST of s128.
[AArch64][RegisterBankInfo] Add mapping support for G_BITCAST of s128
Oct 16 2017, 3:28 PM
qcolombet committed rL315945: [AArch64][LegalizerInfo] Mark s128 G_BITCAST legal.
[AArch64][LegalizerInfo] Mark s128 G_BITCAST legal
Oct 16 2017, 3:28 PM
qcolombet accepted D35816: [Greedy RegAlloc] Add logic to greedy reg alloc to avoid bad eviction chains.

Hi Marina,

Oct 16 2017, 8:57 AM

Oct 13 2017

qcolombet committed rL315781: [AArch64][RegisterBankInfo] Use the statically computed mappings for COPY.
[AArch64][RegisterBankInfo] Use the statically computed mappings for COPY
Oct 13 2017, 5:43 PM
qcolombet added inline comments to D36569: [globalisel][tablegen] Add support for fpimm and import of APInt/APFloat based ImmLeaf..
Oct 13 2017, 2:22 PM
qcolombet committed rL315759: [RegisterBankInfo] Cache the getMinimalPhysRegClass information.
[RegisterBankInfo] Cache the getMinimalPhysRegClass information
Oct 13 2017, 2:16 PM
qcolombet committed rL315758: [Legalizer] Use SmallSetVector instead of SetVector..
[Legalizer] Use SmallSetVector instead of SetVector.
Oct 13 2017, 2:16 PM
qcolombet committed rL315757: [LegalizerInfo] Don't evaluate end boundary every time through the loop.
[LegalizerInfo] Don't evaluate end boundary every time through the loop
Oct 13 2017, 2:16 PM
qcolombet committed rL315756: [Legalizer] Only allocate the SetVectors once per function..
[Legalizer] Only allocate the SetVectors once per function.
Oct 13 2017, 2:16 PM

Oct 12 2017

qcolombet accepted D37445: [globalisel][tablegen] Map ld and st to G_LOAD and G_STORE. NFC.

LGTM with the previous nitpicks fixed.

Oct 12 2017, 2:21 PM
qcolombet added inline comments to D36569: [globalisel][tablegen] Add support for fpimm and import of APInt/APFloat based ImmLeaf..
Oct 12 2017, 2:16 PM
qcolombet accepted D37458: [globalisel][tblgen] Add support for iPTR and implement am_unscaled* and am_indexed*.

Looks good modulo what I said in https://reviews.llvm.org/D37457 regarding GIM_CheckPointerToAny

Oct 12 2017, 12:55 PM
qcolombet added inline comments to D37457: [globalisel][tablegen] Implement unindexed load, non-extending load, and MemVT checks.
Oct 12 2017, 12:45 PM
qcolombet accepted D37456: [globalisel][tablegen] Import ComplexPattern when used as an operator.

Looks fine with one comment:
Do you think we need a new opcode in the state machine for that?

Oct 12 2017, 12:35 PM
qcolombet added a comment to D37445: [globalisel][tablegen] Map ld and st to G_LOAD and G_STORE. NFC.

The added predicate part looks fine, but I didn't get why we need to change CodeGenInstruction into record just yet.

Oct 12 2017, 12:26 PM
qcolombet added a comment to D37443: [tablegen] Handle common load/store predicates inside tablegen. NFC..

Looks mostly good to me.
Comments below.

Oct 12 2017, 12:06 PM
qcolombet added a comment to D36618: [globalisel][tablegen] Simplify named operand/operator lookups and fix a wrong-code bug this revealed..

Looks mostly good.
Couple of comments below.

Oct 12 2017, 11:39 AM
qcolombet accepted D36569: [globalisel][tablegen] Add support for fpimm and import of APInt/APFloat based ImmLeaf..

LGTM with nitpicks.

Oct 12 2017, 11:27 AM
qcolombet accepted D36534: [aarch64] Support APInt and APFloat in ImmLeaf subclasses and make AArch64 use them..

LGTM

Oct 12 2017, 11:17 AM

Oct 11 2017

qcolombet added a comment to D38703: [lit] Only enable LSan on darwin when clang supports it.

The problem is fixed.

Oct 11 2017, 11:30 AM
qcolombet added a comment to D38703: [lit] Only enable LSan on darwin when clang supports it.

I will.
Thanks!

Oct 11 2017, 11:25 AM

Oct 10 2017

qcolombet added a comment to D38200: [GISel]: Process new insts to legalize in the order they were created.

I'm a bit confused by the above. Should we allow legalization to look at other instructions, or should we not?

Oct 10 2017, 9:29 AM
qcolombet added inline comments to D38128: Handle COPYs of physregs better (regalloc hints).
Oct 10 2017, 8:17 AM

Oct 9 2017

qcolombet accepted D37640: [GISel]: Fix generation of illegal COPYs (of different sizes) during CallLowering .

LGTM

Oct 9 2017, 11:50 AM
qcolombet accepted D38128: Handle COPYs of physregs better (regalloc hints).

LGTM for the generic part.

Oct 9 2017, 11:37 AM

Oct 6 2017

qcolombet added a comment to D38597: [PEI] Remove required properties and use 'if' instead of std::function.

Sorry Reid @rnk, I didn't know you were waiting for me. I saw already two people were saying it's good and I thought we didn't need a third one :).

Oct 6 2017, 1:40 PM
qcolombet accepted D38622: [GlobalISel] Fix legalizer trying to process a deleted instruction.

LGTM with one comment: could we have the whole pattern being a helper function?

Oct 6 2017, 10:20 AM
qcolombet accepted D38621: [AArch64][GlobalISel] Make G_PHI of p0 types legal.

Hi Amara,

Oct 6 2017, 10:17 AM

Oct 3 2017

qcolombet added a comment to D38482: TargetMachine: Use LLVMTargetMachine in CodeGen.

I think we should probably just merge these.

Oct 3 2017, 3:24 PM

Oct 2 2017

qcolombet committed rL314760: [Legalizer] Add support for G_OR NarrowScalar..
[Legalizer] Add support for G_OR NarrowScalar.
Oct 2 2017, 9:55 PM

Sep 27 2017

qcolombet added a comment to D37899: [SystemZ] Implement shouldCoalesce() to help regalloc to avoid running out of registers with GR128 regs.

This patch introduces an extra parameter to shouldCoalesce() (LiveIntervals*). Does this look acceptable?

Looks fine to me.

Sep 27 2017, 1:52 PM
qcolombet accepted D38309: [RegAllocGreedy]: Allow recoloring of done register if it's non-tied.

Looks sensible to me.

Sep 27 2017, 1:49 PM

Sep 25 2017

qcolombet committed rL314168: [GlobalISel] Update the documentation and comment for G_[UN]MERGE_VALUES.
[GlobalISel] Update the documentation and comment for G_[UN]MERGE_VALUES
Sep 25 2017, 3:04 PM
qcolombet committed rL314166: [GlobalISel] Update the documentation and comments for G_EXTRACT.
[GlobalISel] Update the documentation and comments for G_EXTRACT
Sep 25 2017, 3:04 PM
qcolombet committed rL314167: [GlobalISel] Update the documentation for G_SEQUENCE.
[GlobalISel] Update the documentation for G_SEQUENCE
Sep 25 2017, 3:04 PM
qcolombet added a comment to D30529: [RFC][GlobalISel] Enable legalizing non-power-of-2 sized types..

Hi Kristof,

Sep 25 2017, 10:31 AM

Sep 21 2017

qcolombet added a comment to D38128: Handle COPYs of physregs better (regalloc hints).

calculateSpillWeightAndHint, why is this not working here?

Sep 21 2017, 10:28 AM

Sep 20 2017

qcolombet committed rL313774: [InstCombine] Add select simplifications.
[InstCombine] Add select simplifications
Sep 20 2017, 10:34 AM
qcolombet closed D37019: Add select simplifications by committing rL313774: [InstCombine] Add select simplifications.
Sep 20 2017, 10:34 AM

Sep 19 2017

qcolombet committed rL313696: [MIRPrinter] Print empty successor lists when they cannot be guessed.
[MIRPrinter] Print empty successor lists when they cannot be guessed
Sep 19 2017, 4:36 PM
qcolombet committed rL313686: Revert "[MIRPrinter] Print empty successor lists when they cannot be guessed".
Revert "[MIRPrinter] Print empty successor lists when they cannot be guessed"
Sep 19 2017, 3:05 PM
qcolombet committed rL313685: [MIRPrinter] Print empty successor lists when they cannot be guessed.
[MIRPrinter] Print empty successor lists when they cannot be guessed
Sep 19 2017, 2:57 PM

Sep 18 2017

qcolombet accepted D37775: Add a verifier test to check the access on both sides of COPY are the same.
Sep 18 2017, 10:51 AM
qcolombet added a comment to D35816: [Greedy RegAlloc] Add logic to greedy reg alloc to avoid bad eviction chains.

For the record, playing with the order helps but to do the optimal (local) coloring here I would need to spend more time on the heuristic.
Again the easiest fix is probably the hints reconciling by region.

Sep 18 2017, 10:46 AM
qcolombet added a comment to D37019: Add select simplifications.

@majnemer
Hi David,

Sep 18 2017, 9:50 AM

Sep 15 2017

qcolombet accepted D37775: Add a verifier test to check the access on both sides of COPY are the same.

LGTM with one suggestion

Sep 15 2017, 2:28 PM

Sep 14 2017

qcolombet added a comment to D35816: [Greedy RegAlloc] Add logic to greedy reg alloc to avoid bad eviction chains.

Hi Marina,

Sep 14 2017, 5:29 PM

Sep 13 2017

qcolombet added inline comments to D37640: [GISel]: Fix generation of illegal COPYs (of different sizes) during CallLowering .
Sep 13 2017, 2:56 PM
qcolombet added a comment to D37640: [GISel]: Fix generation of illegal COPYs (of different sizes) during CallLowering .

The AArch64 part looks good to me.

Sep 13 2017, 11:22 AM
qcolombet added inline comments to D37775: Add a verifier test to check the access on both sides of COPY are the same.
Sep 13 2017, 10:42 AM
qcolombet added inline comments to D37775: Add a verifier test to check the access on both sides of COPY are the same.
Sep 13 2017, 9:36 AM

Sep 12 2017

qcolombet accepted D37019: Add select simplifications.

Thanks Michael.

Sep 12 2017, 2:40 PM

Sep 11 2017

qcolombet added inline comments to D37019: Add select simplifications.
Sep 11 2017, 4:03 PM
qcolombet added a comment to D31834: Remove unnecessary bitvector clear.

Sorry, that one slipped through!

Sep 11 2017, 4:00 PM
qcolombet added inline comments to D37019: Add select simplifications.
Sep 11 2017, 3:50 PM
qcolombet added inline comments to D37019: Add select simplifications.
Sep 11 2017, 3:46 PM
qcolombet added a comment to D37640: [GISel]: Fix generation of illegal COPYs (of different sizes) during CallLowering .

Hi Aditya,

Sep 11 2017, 12:15 PM
qcolombet accepted D36795: [SystemZ] Increase number of LOCRs emitted by passing regalloc hints.

Do you think that looking at the number of spilled live ranges is a good way to consider the trade-off for using hard hints, or do you perhaps have anything to add?

Sep 11 2017, 9:53 AM

Sep 8 2017

qcolombet added a comment to D36795: [SystemZ] Increase number of LOCRs emitted by passing regalloc hints.

The generic change looks fine though it sounds surprising that anyone would want that.

Sep 8 2017, 12:48 PM
qcolombet accepted D37578: [RegAlloc] Keep a copy of live interval for the spilled vregs in HoistSpillHelper.

LGTM with a test case (.mir preferably) from PR34502

Sep 8 2017, 11:04 AM

Aug 25 2017

qcolombet accepted D36913: TableGen: Fix subreg composition/concatenation.

LGTM.
Nitpicks below, no need to submit a new version.

Aug 25 2017, 10:42 AM
qcolombet accepted D36911: TableGen: Add --gen-register-info-debug-dump.

LGTM too.
Same comment as Krzysztof, would be nice if the debug printing was more intuitively discoverable (e.g., -debug alongside with -gen-register-info).

Aug 25 2017, 10:19 AM

Aug 24 2017

qcolombet added inline comments to D37065: Ensure standard pseudo instructions (TargetOpcode::*) are compatible with guessInstructionProperties=0.
Aug 24 2017, 5:02 PM
qcolombet accepted D37097: Set hasSideEffects=0 for PHI and fix passes relying isSafeToMove/hasUnmodeledSideEffects being true for PHI.

LGTM

Aug 24 2017, 4:59 PM
qcolombet accepted D37018: [GISel]: Legalize G_PHIs.

Few more nitpicks, no need for more reviews.

Aug 24 2017, 4:45 PM
qcolombet added a comment to D37018: [GISel]: Legalize G_PHIs.

I miss these test cases:

  • Add a test case with def feeding into two different phis:
    • In the same destination block
    • In different basic block (one on the true branch, one on the false branch)
Aug 24 2017, 2:27 PM
qcolombet added a comment to D37018: [GISel]: Legalize G_PHIs.

Hi Aditya,

Aug 24 2017, 12:41 PM
qcolombet added inline comments to D37097: Set hasSideEffects=0 for PHI and fix passes relying isSafeToMove/hasUnmodeledSideEffects being true for PHI.
Aug 24 2017, 10:47 AM

Aug 23 2017

qcolombet added inline comments to D37065: Ensure standard pseudo instructions (TargetOpcode::*) are compatible with guessInstructionProperties=0.
Aug 23 2017, 2:59 PM
qcolombet accepted D36990: [GISel]: Translate phi into generic G_PHI instruction.

LGTM

Aug 23 2017, 11:07 AM
qcolombet added inline comments to D37065: Ensure standard pseudo instructions (TargetOpcode::*) are compatible with guessInstructionProperties=0.
Aug 23 2017, 10:28 AM
qcolombet added a comment to D35816: [Greedy RegAlloc] Add logic to greedy reg alloc to avoid bad eviction chains.

Hi Marina,

Aug 23 2017, 10:24 AM
qcolombet added inline comments to D36990: [GISel]: Translate phi into generic G_PHI instruction.
Aug 23 2017, 10:20 AM
qcolombet added inline comments to D37065: Ensure standard pseudo instructions (TargetOpcode::*) are compatible with guessInstructionProperties=0.
Aug 23 2017, 9:45 AM

Aug 22 2017

qcolombet added a comment to D37016: [GISel]: Allow the MachineIRBuilder to insert instructions after.

Good point. I think this is not needed. I'll update the other patch to use the std::next version. We can add this back at a later time if it's of value.

You can probably directly use setInsertPt with getFirstTerminator I think (i.e., no need to use std::next ;))

Aug 22 2017, 4:45 PM
qcolombet added a comment to D37016: [GISel]: Allow the MachineIRBuilder to insert instructions after.

Note: Given the other review, I let you decide if we need that patch or not.

Aug 22 2017, 4:33 PM
qcolombet added a comment to D37018: [GISel]: Legalize G_PHIs.

Hi Aditya,

Aug 22 2017, 4:31 PM
qcolombet added a comment to D36990: [GISel]: Translate phi into generic G_PHI instruction.

Hi Aditya,

Aug 22 2017, 4:17 PM
qcolombet added a comment to D37016: [GISel]: Allow the MachineIRBuilder to insert instructions after.

Just one nit.

Aug 22 2017, 4:13 PM
qcolombet accepted D37016: [GISel]: Allow the MachineIRBuilder to insert instructions after.

I believe we use to do that by using std::next([Instr | Iterator]WeWantToInsertAfter), but I can see how that could make things easier, so LGTM

Aug 22 2017, 4:09 PM