Page MenuHomePhabricator

qcolombet (Quentin Colombet)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 17 2012, 10:03 AM (381 w, 2 d)

Recent Activity

Tue, Mar 31

qcolombet added inline comments to D76640: [GlobalISel] Combine (x op 0) -> x for operations with a right identity of 0.
Tue, Mar 31, 10:52 AM · Restricted Project
qcolombet added a comment to D77105: [GlobalISel] Implement identity transforms for x op x -> x.

Where do these patterns come from?
I would expect they would be eliminated by InstCombine in the IR.

Tue, Mar 31, 9:24 AM · Restricted Project

Tue, Mar 10

qcolombet accepted D74919: AMDGPU/GlobalISel: Manually RegBankSelect copies.
Tue, Mar 10, 9:33 PM · Restricted Project

Mar 9 2020

qcolombet accepted D75830: Move Spiller.h from lib/ directory path to include/CodeGen. NFC.
Mar 9 2020, 9:41 AM · Restricted Project

Feb 27 2020

qcolombet accepted D75137: GlobalISel: Move some legalizer functions to utils.

LGTM

Feb 27 2020, 10:28 AM · Restricted Project

Feb 26 2020

qcolombet added a comment to D75086: [AArch64][GlobalISel] Fixup <32b heterogeneous regbanks of G_PHIs just before selection..

I understand that cross copies are bad, but what I don't understand is why don't we form an extended load? That would apply nicely to your example.

Feb 26 2020, 6:34 PM · Restricted Project
qcolombet added a comment to D75086: [AArch64][GlobalISel] Fixup <32b heterogeneous regbanks of G_PHIs just before selection..

Something just stroke me, why don't you use the FPR bank for all 16-bit values then?

Feb 26 2020, 1:51 PM · Restricted Project
qcolombet accepted D75086: [AArch64][GlobalISel] Fixup <32b heterogeneous regbanks of G_PHIs just before selection..

s16 operations, if legal, just get selected to gpr32 regclass instructions. E.g.

%ld:gpr(s16) = G_LOAD ...
=>
%ld:gpr32 = LDR_16 ...
Feb 26 2020, 11:59 AM · Restricted Project
qcolombet added a comment to D75086: [AArch64][GlobalISel] Fixup <32b heterogeneous regbanks of G_PHIs just before selection..

Thanks for the clarification Amara.

Feb 26 2020, 10:25 AM · Restricted Project

Feb 25 2020

qcolombet committed rG5bf0023b0d70: [GISel][KnownBits] Update a comment regarding the effect of cache on PHIs (authored by qcolombet).
[GISel][KnownBits] Update a comment regarding the effect of cache on PHIs
Feb 25 2020, 3:59 PM
qcolombet added a comment to D75086: [AArch64][GlobalISel] Fixup <32b heterogeneous regbanks of G_PHIs just before selection..

Hi Amara,

Feb 25 2020, 2:37 PM · Restricted Project
qcolombet committed rGa12f1d6a52a1: [MachineInstr] Add a dumpr method (authored by qcolombet).
[MachineInstr] Add a dumpr method
Feb 25 2020, 10:47 AM
qcolombet closed D75094: [MachineInstr] Add a dumpr method.
Feb 25 2020, 10:47 AM · Restricted Project

Feb 24 2020

qcolombet updated the diff for D75094: [MachineInstr] Add a dumpr method.
  • Handle PhysReg and Non-SSA code in general
Feb 24 2020, 8:48 PM · Restricted Project
qcolombet added a comment to D75094: [MachineInstr] Add a dumpr method.

Here are a few examples of the dump we get:

  • No Depth limit:
(lldb) p MI.dumpr(MRI, -1)
%1:_(<4 x s16>) = G_BUILD_VECTOR %6:_(s16), %7:_(s16), %8:_(s16), %9:_(s16)
  %6:_(s16) = G_FPTRUNC %17:_(s32)
    %17:_(s32) = COPY $s0
  %7:_(s16) = G_FPTRUNC %15:_(s32)
    %15:_(s32) = COPY $s0
  %8:_(s16) = G_FPTRUNC %13:_(s32)
    %13:_(s32) = COPY $s0
  %9:_(s16) = G_FPTRUNC %11:_(s32)
    %11:_(s32) = COPY $s0
Feb 24 2020, 8:48 PM · Restricted Project
qcolombet updated the diff for D75094: [MachineInstr] Add a dumpr method.
  • Use UINT_MAX instead of Optional to ease the use of the method in the debugger
  • Use the dedicated formatted_raw_ostream to pad column
Feb 24 2020, 8:39 PM · Restricted Project
qcolombet added inline comments to D75094: [MachineInstr] Add a dumpr method.
Feb 24 2020, 8:04 PM · Restricted Project
qcolombet created D75094: [MachineInstr] Add a dumpr method.
Feb 24 2020, 5:40 PM · Restricted Project

Feb 21 2020

qcolombet committed rGb6d63c92ec37: [GISel][KnownBits] Suppress unused warning on the dump method (authored by qcolombet).
[GISel][KnownBits] Suppress unused warning on the dump method
Feb 21 2020, 9:16 PM
qcolombet added a comment to D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.

Does that match your view of our conversation?

Feb 21 2020, 3:07 PM · Restricted Project
qcolombet closed D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.

To https://github.com/llvm/llvm-project.git

9708279c725..618dec2aeff  master -> master
Feb 21 2020, 3:07 PM · Restricted Project
qcolombet added a comment to D69868: Allow "callbr" to return non-void values.

I've been concerned about the register live-ins too (I'm less concerned about the successors issue). Is there documentation on the original decision to disallow physical register live-ins for MBBs before register allocation? We could then check to see if we're violating the original reasoning.

Feb 21 2020, 2:57 PM · Restricted Project, Restricted Project
qcolombet committed rG618dec2aeffd: [GISel][KnownBits] Add a cache mechanism to speed compile time (authored by qcolombet).
[GISel][KnownBits] Add a cache mechanism to speed compile time
Feb 21 2020, 2:39 PM
qcolombet updated the diff for D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.
  • Use SmallDenseMap instead of DenseMap
Feb 21 2020, 2:02 PM · Restricted Project
qcolombet added a comment to D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.

I think the argument that because it's possible to do the wrong thing if you try hard enough that you have to protect against that is rather flawed and inconsistent with the existing code in LLVM.

Feb 21 2020, 1:25 PM · Restricted Project
qcolombet added inline comments to D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.
Feb 21 2020, 1:25 PM · Restricted Project
qcolombet added a comment to D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.

That's true (although you'd need a third vreg with no defs/uses to do it) but as you say there's no reason to do so.

Feb 21 2020, 12:04 PM · Restricted Project
qcolombet added a comment to D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.

My long term plan for the intra/inter-pass cache is based about caching based on the register too.

Feb 21 2020, 11:36 AM · Restricted Project
qcolombet added inline comments to D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.
Feb 21 2020, 10:23 AM · Restricted Project
qcolombet updated the diff for D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.

Switch the caching to register instead of instruction.

Feb 21 2020, 10:15 AM · Restricted Project

Feb 20 2020

qcolombet retitled D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time from [GISel][KnownBits] Add a cache mechanism to speed compile time to [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.
Feb 20 2020, 6:24 PM · Restricted Project
qcolombet created D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.
Feb 20 2020, 6:15 PM · Restricted Project
qcolombet committed rGe4a9225f5d10: [GISel][KnownBits] Give up on PHI analysis as soon as we don't know anything (authored by qcolombet).
[GISel][KnownBits] Give up on PHI analysis as soon as we don't know anything
Feb 20 2020, 11:43 AM

Feb 10 2020

qcolombet accepted D54482: RegisterCoalescer: Add LaneMask to debug printing.
Feb 10 2020, 12:00 PM

Feb 5 2020

qcolombet accepted D73152: [PHIElimination] Compile time optimization for huge functions..

LGTM with a nit. Use a reference instead of a pointer (or check the pointer and fallback to the old addNewBlock when the pointer is nullptr)

Feb 5 2020, 1:41 PM · Restricted Project

Feb 3 2020

qcolombet committed rGf26ff8c9df79: [TargetRegisterInfo] Make the heuristic to skip region split overridable by the… (authored by qcolombet).
[TargetRegisterInfo] Make the heuristic to skip region split overridable by the…
Feb 3 2020, 11:39 AM

Jan 30 2020

qcolombet committed rGcfebd7774229: [GISel][KnownBits] Fix a bug where we could run out of stack space (authored by qcolombet).
[GISel][KnownBits] Fix a bug where we could run out of stack space
Jan 30 2020, 7:52 PM

Jan 27 2020

qcolombet added a comment to D73317: [GISelKnownBits] Add support for PHIs.

and having the builder silently ignore the explicitly setInsertPt. I think that behavior should be opt-in only to CSE, and obvious when it's being done.

Jan 27 2020, 9:51 AM · Restricted Project

Jan 24 2020

qcolombet committed rG5d87b5d20296: [GISelKnownBits] Add support for PHIs (authored by qcolombet).
[GISelKnownBits] Add support for PHIs
Jan 24 2020, 4:45 PM
qcolombet closed D73317: [GISelKnownBits] Add support for PHIs.
Jan 24 2020, 4:45 PM · Restricted Project
qcolombet added a comment to D73317: [GISelKnownBits] Add support for PHIs.

Thanks @arsenm for the review!

Jan 24 2020, 3:59 PM · Restricted Project
qcolombet updated the diff for D73317: [GISelKnownBits] Add support for PHIs.
  • Fix typo in comment
  • Don't increase depth on COPYs
  • Use Register instead of unsigned in added unitests
Jan 24 2020, 9:08 AM · Restricted Project
qcolombet added a comment to D73317: [GISelKnownBits] Add support for PHIs.

I do think copies should be ignored for depth. Other places generally try to treat intervening copies as irrelevant

Jan 24 2020, 8:58 AM · Restricted Project

Jan 23 2020

qcolombet added inline comments to D73317: [GISelKnownBits] Add support for PHIs.
Jan 23 2020, 5:56 PM · Restricted Project
qcolombet created D73317: [GISelKnownBits] Add support for PHIs.
Jan 23 2020, 5:56 PM · Restricted Project
qcolombet accepted D73280: [LoopUnroll] Avoid UB when converting from WeakVH to `Value *`.
Jan 23 2020, 10:36 AM · Restricted Project

Jan 21 2020

qcolombet committed rGff1f3cc1a12e: [GISelKnownBits] Make the max depth a parameter of the analysis (authored by qcolombet).
[GISelKnownBits] Make the max depth a parameter of the analysis
Jan 21 2020, 11:37 AM

Dec 16 2019

qcolombet added a comment to D71176: [GlobalISel] Delete unused TargetPassConfig::addPreRegBankSelect.

I would rather we keep it. Although like you said we could piggyback on addRegBankSelect, it is clearer to have a separate method and that's consistent with the other GISel passes.

Dec 16 2019, 3:57 PM · Restricted Project

Dec 5 2019

qcolombet committed rG2ec71ea7c74d: [RegisterCoalescer] Fix the creation of subranges when rematerialization is used (authored by qcolombet).
[RegisterCoalescer] Fix the creation of subranges when rematerialization is used
Dec 5 2019, 4:35 PM

Dec 4 2019

qcolombet accepted D71038: [GlobalISel] Localizer: Allow targets not to run the pass conditionally.
Dec 4 2019, 5:29 PM · Restricted Project

Dec 3 2019

qcolombet accepted D67794: [MachineCopyPropagation] Extend MCP to do trivial copy backward propagation.

LGTM.
One nit below.

Dec 3 2019, 9:00 AM · Restricted Project

Dec 2 2019

qcolombet added inline comments to D70794: [GlobalISel] Fix compiler crash lowering G_LOAD in AArch64..
Dec 2 2019, 5:35 PM · Restricted Project
qcolombet added inline comments to D67794: [MachineCopyPropagation] Extend MCP to do trivial copy backward propagation.
Dec 2 2019, 5:26 PM · Restricted Project
qcolombet added inline comments to D67794: [MachineCopyPropagation] Extend MCP to do trivial copy backward propagation.
Dec 2 2019, 4:48 PM · Restricted Project
qcolombet accepted D70565: [MCRegInfo] Add forward sub and super register iterators. (NFC).
Dec 2 2019, 1:57 PM · Restricted Project
qcolombet added a comment to D69936: [IPRA][ARM] Spill extra registers at -Oz.

Looks reasonable (haven't looked at the ARM part.)
The change in shrink wrapping doesn't look correct to me though. Is that particular change require?

Dec 2 2019, 1:57 PM · Restricted Project

Nov 21 2019

qcolombet accepted D70558: [MIBundles] Move analyzeVirtReg out of MIBundleOperands iterator (NFC)..

LGTM

Nov 21 2019, 12:59 PM · Restricted Project

Nov 15 2019

qcolombet committed rG98ceac498167: [GISel][CombinerHelper] Use uses() instead of operands() when traversing use… (authored by qcolombet).
[GISel][CombinerHelper] Use uses() instead of operands() when traversing use…
Nov 15 2019, 1:57 PM
qcolombet committed rG304abde0779b: [GISel][CombinerHelper] Add support for scalar type for the result of shuffle… (authored by qcolombet).
[GISel][CombinerHelper] Add support for scalar type for the result of shuffle…
Nov 15 2019, 1:57 PM

Nov 13 2019

qcolombet committed rGde94cda81bde: [LiveInterval] Allow updating subranges with slightly out-dated IR (authored by qcolombet).
[LiveInterval] Allow updating subranges with slightly out-dated IR
Nov 13 2019, 11:24 AM

Nov 6 2019

qcolombet committed rG52af7aedfe5d: [GISel][ArtifactCombiner] Relax the constraint to combine unmerge with… (authored by qcolombet).
[GISel][ArtifactCombiner] Relax the constraint to combine unmerge with…
Nov 6 2019, 11:33 AM
qcolombet closed D69288: [GISel][ArtifactCombiner] Relax the constraint to combine unmerge with concat_vectors.
Nov 6 2019, 11:32 AM · Restricted Project

Nov 4 2019

qcolombet added a comment to D69288: [GISel][ArtifactCombiner] Relax the constraint to combine unmerge with concat_vectors.

Are you planning on fixing the regression in the near future? If so splitting the patches this way seems fine

Nov 4 2019, 11:26 AM · Restricted Project
qcolombet added a comment to D69288: [GISel][ArtifactCombiner] Relax the constraint to combine unmerge with concat_vectors.

@arsenm ping^2 on the AMDGPU test changes.

Nov 4 2019, 9:03 AM · Restricted Project

Nov 1 2019

qcolombet accepted D68267: [MBB LiveIn lists, MachineVerifier, SystemZ] New method isLiveOut() and mverifier improvement..

LGTM, thanks!

Nov 1 2019, 9:39 AM · Restricted Project
qcolombet added a comment to D69653: [GlobalISel] Match table opt: fix a bug in matching num of operands.

Thanks for the test case!

Nov 1 2019, 9:29 AM · Restricted Project

Oct 31 2019

qcolombet accepted D69653: [GlobalISel] Match table opt: fix a bug in matching num of operands.

For the test case, couldn't you add a isVariadic instruction with different patterns in one of the TableGen test?

Oct 31 2019, 1:54 PM · Restricted Project
qcolombet added inline comments to D67794: [MachineCopyPropagation] Extend MCP to do trivial copy backward propagation.
Oct 31 2019, 10:29 AM · Restricted Project
qcolombet added a comment to D68267: [MBB LiveIn lists, MachineVerifier, SystemZ] New method isLiveOut() and mverifier improvement..

Looks sensible to me.

Oct 31 2019, 10:20 AM · Restricted Project

Oct 30 2019

qcolombet committed rGf0eeb3c7a713: [GISel][CombinerHelper] Combine shuffle_vector scalar to build_vector (authored by qcolombet).
[GISel][CombinerHelper] Combine shuffle_vector scalar to build_vector
Oct 30 2019, 6:41 PM

Oct 29 2019

qcolombet added a comment to D69288: [GISel][ArtifactCombiner] Relax the constraint to combine unmerge with concat_vectors.

@arsenm ping on the AMDGPU test changes.

Oct 29 2019, 11:13 AM · Restricted Project

Oct 28 2019

qcolombet added a comment to D69219: [SelectionDAG] Enable lowering unordered atomics loads w/LoadSDNode (and stores w/StoreSDNode) by default.

Thanks Philip for double checking.

Oct 28 2019, 3:01 PM · Restricted Project
qcolombet requested changes to D67794: [MachineCopyPropagation] Extend MCP to do trivial copy backward propagation.
Oct 28 2019, 12:56 PM · Restricted Project
qcolombet accepted D69219: [SelectionDAG] Enable lowering unordered atomics loads w/LoadSDNode (and stores w/StoreSDNode) by default.
Oct 28 2019, 11:56 AM · Restricted Project
qcolombet accepted D69515: [LiveIntervalUnion] Expose extraction of last index in map for external users.
Oct 28 2019, 8:54 AM · Restricted Project

Oct 25 2019

qcolombet accepted D69449: TableGen: Use enum names in composeSubRegIndices table.
Oct 25 2019, 2:56 PM · Restricted Project

Oct 24 2019

qcolombet added a comment to D69288: [GISel][ArtifactCombiner] Relax the constraint to combine unmerge with concat_vectors.

I found a latent bug that has now more chances to trigger with the relaxations that this patch brings.
I've updated the patch to fix the bug, but I have now a bunch of "regressions" on AMDGPU test cases because the fix is too conservative.
Is this still okay to land?

Oct 24 2019, 10:37 PM · Restricted Project
qcolombet added inline comments to D69288: [GISel][ArtifactCombiner] Relax the constraint to combine unmerge with concat_vectors.
Oct 24 2019, 10:37 PM · Restricted Project
qcolombet updated the diff for D69288: [GISel][ArtifactCombiner] Relax the constraint to combine unmerge with concat_vectors.
  • Fix a latent bug
  • Update AMDGPU tests
Oct 24 2019, 10:18 PM · Restricted Project

Oct 23 2019

qcolombet updated the diff for D69288: [GISel][ArtifactCombiner] Relax the constraint to combine unmerge with concat_vectors.
  • Remove ADJUSTxxx operations in the tests
Oct 23 2019, 8:23 AM · Restricted Project

Oct 22 2019

qcolombet added inline comments to D69288: [GISel][ArtifactCombiner] Relax the constraint to combine unmerge with concat_vectors.
Oct 22 2019, 8:09 AM · Restricted Project

Oct 21 2019

qcolombet created D69288: [GISel][ArtifactCombiner] Relax the constraint to combine unmerge with concat_vectors.
Oct 21 2019, 5:29 PM · Restricted Project
qcolombet added a comment to D69149: [GISel][CombinerHelper] Add a combine turning shuffle_vector into concat_vectors.

Thanks for the quick review @arsenm!

Oct 21 2019, 2:04 PM · Restricted Project
qcolombet committed rG6f0ae81512c1: [GISel][CombinerHelper] Add a combine turning shuffle_vector into concat_vectors (authored by qcolombet).
[GISel][CombinerHelper] Add a combine turning shuffle_vector into concat_vectors
Oct 21 2019, 1:46 PM
qcolombet closed D69149: [GISel][CombinerHelper] Add a combine turning shuffle_vector into concat_vectors.
Oct 21 2019, 1:46 PM · Restricted Project
qcolombet committed rL375452: [GISel][CombinerHelper] Add a combine turning shuffle_vector into concat_vectors.
[GISel][CombinerHelper] Add a combine turning shuffle_vector into concat_vectors
Oct 21 2019, 1:46 PM

Oct 18 2019

qcolombet accepted D68149: LiveIntervals: Fix handleMoveUp with subreg def moving across a def.

LGTM.

Oct 18 2019, 2:41 PM
qcolombet committed rG9f9151d49410: [GISel][CallLowering] Make isIncomingArgumentHandler a pure virtual method (authored by qcolombet).
[GISel][CallLowering] Make isIncomingArgumentHandler a pure virtual method
Oct 18 2019, 1:17 PM
qcolombet committed rL375277: [GISel][CallLowering] Make isIncomingArgumentHandler a pure virtual method.
[GISel][CallLowering] Make isIncomingArgumentHandler a pure virtual method
Oct 18 2019, 1:17 PM
qcolombet closed D69187: [GISel][CallLowering] Make isIncomingArgumentHandler a pure virtual method.
Oct 18 2019, 1:17 PM · Restricted Project
qcolombet created D69187: [GISel][CallLowering] Make isIncomingArgumentHandler a pure virtual method.
Oct 18 2019, 11:25 AM · Restricted Project

Oct 17 2019

qcolombet updated the diff for D69149: [GISel][CombinerHelper] Add a combine turning shuffle_vector into concat_vectors.
  • Replace some unsigned into Register
Oct 17 2019, 4:40 PM · Restricted Project
qcolombet added inline comments to D69149: [GISel][CombinerHelper] Add a combine turning shuffle_vector into concat_vectors.
Oct 17 2019, 4:40 PM · Restricted Project
qcolombet abandoned D59339: [GISel][Legalizer][WIP][RFC] Retry legalization for failing instrs when artifacts are around.

This was just a patch to illustrate a possible direction.

Oct 17 2019, 4:31 PM · Restricted Project
qcolombet created D69149: [GISel][CombinerHelper] Add a combine turning shuffle_vector into concat_vectors.
Oct 17 2019, 4:12 PM · Restricted Project

Oct 16 2019

qcolombet committed rGc319afc903d8: [GISel][CombinerHelper] Add concat_vectors(build_vector, build_vector) =>… (authored by qcolombet).
[GISel][CombinerHelper] Add concat_vectors(build_vector, build_vector) =>…
Oct 16 2019, 5:39 PM
qcolombet closed D69071: [GISel][CombinerHelper] Add concat_vectors(build_vector, build_vector) => build_vector.
Oct 16 2019, 5:38 PM · Restricted Project
qcolombet committed rL375066: [GISel][CombinerHelper] Add concat_vectors(build_vector, build_vector) =>….
[GISel][CombinerHelper] Add concat_vectors(build_vector, build_vector) =>…
Oct 16 2019, 5:38 PM
qcolombet updated the diff for D69071: [GISel][CombinerHelper] Add concat_vectors(build_vector, build_vector) => build_vector.
  • Remove asserts already covered by the verifier
  • Put .getType calls into a variable.
Oct 16 2019, 4:48 PM · Restricted Project
qcolombet updated the diff for D69071: [GISel][CombinerHelper] Add concat_vectors(build_vector, build_vector) => build_vector.
  • Replace check with an assertion
  • Update comments with pre-conditions
Oct 16 2019, 3:36 PM · Restricted Project