Page MenuHomePhabricator

qcolombet (Quentin Colombet)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 17 2012, 10:03 AM (322 w, 4 d)

Recent Activity

Yesterday

qcolombet accepted D55283: CodeGen: Refactor regallocator command line and target selection.

LGTM, one last nitpick below.

Fri, Feb 22, 11:58 AM
qcolombet added inline comments to D55283: CodeGen: Refactor regallocator command line and target selection.
Fri, Feb 22, 9:04 AM

Tue, Feb 19

qcolombet added inline comments to D58402: RegBankSelect: Allow targets to introduce control flow for mapping.
Tue, Feb 19, 4:33 PM
qcolombet accepted D58402: RegBankSelect: Allow targets to introduce control flow for mapping.

Nice catch!

Tue, Feb 19, 3:25 PM
qcolombet accepted D58376: [RegAllocGreedy] Take last chance recoloring into account in split and assign.
Tue, Feb 19, 8:32 AM · Restricted Project

Mon, Feb 18

qcolombet accepted D55282: CodeGen: Make RegAllocRegistry a template class.
Mon, Feb 18, 10:42 AM

Thu, Feb 14

qcolombet added a comment to D58252: RegBankSelect: Handle slightly more complex value mappings.

The generic part looks sensible to me.

Thu, Feb 14, 2:37 PM
qcolombet added inline comments to D58242: Teach instcombine about remaining idemptotent atomicrmw types.
Thu, Feb 14, 10:46 AM · Restricted Project
qcolombet added inline comments to D58242: Teach instcombine about remaining idemptotent atomicrmw types.
Thu, Feb 14, 10:29 AM · Restricted Project
qcolombet accepted D55295: LiveIntervals: Add removePhysReg.
Thu, Feb 14, 9:56 AM
qcolombet accepted D58242: Teach instcombine about remaining idemptotent atomicrmw types.
Thu, Feb 14, 9:53 AM · Restricted Project
qcolombet added a comment to D58242: Teach instcombine about remaining idemptotent atomicrmw types.

LGTM, one nitpick below.

Thu, Feb 14, 9:53 AM · Restricted Project

Wed, Feb 13

qcolombet closed D58200: [RegAllocGreedy] Take last chance recoloring into account in evicting..
Wed, Feb 13, 3:56 PM · Restricted Project
qcolombet added inline comments to D58200: [RegAllocGreedy] Take last chance recoloring into account in evicting..
Wed, Feb 13, 2:27 PM · Restricted Project
qcolombet accepted D58200: [RegAllocGreedy] Take last chance recoloring into account in evicting..

Hi Mark,

Wed, Feb 13, 1:53 PM · Restricted Project
qcolombet requested changes to D55295: LiveIntervals: Add removePhysReg.
Wed, Feb 13, 1:28 PM
qcolombet added a comment to D58200: [RegAllocGreedy] Take last chance recoloring into account in evicting..

Hi Mark,

Wed, Feb 13, 1:25 PM · Restricted Project
qcolombet added inline comments to D55295: LiveIntervals: Add removePhysReg.
Wed, Feb 13, 10:51 AM
qcolombet accepted D55295: LiveIntervals: Add removePhysReg.

LGTM with nitpick.

Wed, Feb 13, 10:26 AM

Tue, Feb 12

qcolombet added a comment to D55301: RegAlloc: Allow targets to split register allocation.

Hi Matt,

Tue, Feb 12, 10:05 AM
qcolombet added inline comments to D57854: [InstCombine] Optimize `atomicrmw <op>, 0` into `load atomic` when possible.
Tue, Feb 12, 9:53 AM · Restricted Project

Thu, Feb 7

Herald added a project to D51664: [IR] Lazily number instructions for local dominance queries: Restricted Project.

Hi Reid,

Thu, Feb 7, 4:15 PM · Restricted Project
qcolombet committed rG96f54de8ff5d: [InstCombine] Optimize `atomicrmw <op>, 0` into `load atomic` when possible (authored by qcolombet).
[InstCombine] Optimize `atomicrmw <op>, 0` into `load atomic` when possible
Thu, Feb 7, 1:28 PM
qcolombet committed rL353471: [InstCombine] Optimize `atomicrmw <op>, 0` into `load atomic` when possible.
[InstCombine] Optimize `atomicrmw <op>, 0` into `load atomic` when possible
Thu, Feb 7, 1:27 PM
qcolombet closed D57854: [InstCombine] Optimize `atomicrmw <op>, 0` into `load atomic` when possible.
Thu, Feb 7, 1:27 PM · Restricted Project
qcolombet added inline comments to D57854: [InstCombine] Optimize `atomicrmw <op>, 0` into `load atomic` when possible.
Thu, Feb 7, 10:01 AM · Restricted Project

Wed, Feb 6

qcolombet added a comment to D57854: [InstCombine] Optimize `atomicrmw <op>, 0` into `load atomic` when possible.

Thanks for the link @jfb.

Wed, Feb 6, 4:33 PM · Restricted Project
qcolombet updated the diff for D57854: [InstCombine] Optimize `atomicrmw <op>, 0` into `load atomic` when possible.

Update:

  • Switch to listing valid ordering instead of invalid ones
  • Don't apply the transformation on volatile atomicrmw
Wed, Feb 6, 4:32 PM · Restricted Project
qcolombet added inline comments to D57854: [InstCombine] Optimize `atomicrmw <op>, 0` into `load atomic` when possible.
Wed, Feb 6, 4:20 PM · Restricted Project
qcolombet created D57854: [InstCombine] Optimize `atomicrmw <op>, 0` into `load atomic` when possible.
Wed, Feb 6, 3:07 PM · Restricted Project

Tue, Feb 5

qcolombet added a comment to D57243: GlobalISel: Allow bitcount ops to have different result type.

Though it's worth pointing out that none of those currently build a description of what it wants to do like the legalizer currently does, they just make the change straightaway when the rules match. Removing in LegalizationAction in favour of just making the change brings the legalizer closer to the others.

Tue, Feb 5, 11:04 AM
qcolombet added a comment to D57243: GlobalISel: Allow bitcount ops to have different result type.

The legalizer is already done the way I'm talking about on the rule matching side and performance was marginally better on average than the old setAction() interface.

Tue, Feb 5, 10:21 AM
qcolombet added a comment to D57243: GlobalISel: Allow bitcount ops to have different result type.

Should we revert this change to have the conversation going or am I the only one unhappy with the absence of guarantees for the other operands?

Tue, Feb 5, 9:57 AM
qcolombet added a comment to D57243: GlobalISel: Allow bitcount ops to have different result type.
Personally, I'd like to replace the LegalizationAction with a function pointer that just performs the desired change and have a standard library of actions that can be called for the common changes.
Tue, Feb 5, 9:55 AM
qcolombet added inline comments to D57243: GlobalISel: Allow bitcount ops to have different result type.
Tue, Feb 5, 9:53 AM

Thu, Jan 24

qcolombet accepted D55988: WIP: RegBankSelect: Support some more complex part mappings.

Thanks for your patience Matt!

Thu, Jan 24, 1:54 PM

Jan 23 2019

qcolombet added inline comments to D55988: WIP: RegBankSelect: Support some more complex part mappings.
Jan 23 2019, 1:46 PM
qcolombet accepted D54365: RegAllocFast: Remove early selection loop, the spill calculation will report cost 0 anyway for free regs.
Jan 23 2019, 1:31 PM · Restricted Project

Jan 14 2019

qcolombet added inline comments to D55988: WIP: RegBankSelect: Support some more complex part mappings.
Jan 14 2019, 10:51 AM

Jan 8 2019

qcolombet accepted D54551: RegisterCoalescer: Assume CR_Replace for SubRangeJoin.

LGTM

Jan 8 2019, 11:01 AM
qcolombet accepted D54615: RegisterCoalescer: Defer clearing implicit_def lanes.

LGTM

Jan 8 2019, 10:51 AM
qcolombet added inline comments to D55988: WIP: RegBankSelect: Support some more complex part mappings.
Jan 8 2019, 10:43 AM

Jan 7 2019

qcolombet added inline comments to D55988: WIP: RegBankSelect: Support some more complex part mappings.
Jan 7 2019, 2:26 PM
qcolombet accepted D55867: [RegisterCoalescer] dst register's live interval needs to be updated when merging a src register in ToBeUpdated set.

LGTM

Jan 7 2019, 2:07 PM

Dec 21 2018

qcolombet added a comment to D55988: WIP: RegBankSelect: Support some more complex part mappings.

Hi Matt,

Dec 21 2018, 2:52 PM

Dec 20 2018

qcolombet added a comment to D55909: [ARM] Set Defs = [CPSR] for COPY_STRUCT_BYVAL, as it clobbers CPSR..

Could we just say that ExpandISelPseudo doesn't preserve the liveness and have it be recomputed when needed?

Dec 20 2018, 5:13 PM
qcolombet added a comment to D51362: [GlobalISel][IRTranslator] Canonicalize G_ICMP to have constant operands last.

I still believe we shouldn't do that here and more generally O0 should be kept out of doing things on the code.

Dec 20 2018, 5:09 PM
qcolombet added a comment to D55775: [Driver] Don't override '-march' when using '-arch x86_64h'.

Should we emit an error if we request x86_64h with an arch older than haswell?

Dec 20 2018, 5:03 PM
qcolombet accepted D55911: GlobalISel: Correct example PartialMapping table.

LGTM

Dec 20 2018, 4:35 PM
qcolombet accepted D55872: RegBankSelect: Fix copy insertion point for terminators.

I thought about this, but figured since in this case it's most likely for not really copyable condition registers it might end up being problematic

Dec 20 2018, 4:34 PM
qcolombet added inline comments to D55909: [ARM] Set Defs = [CPSR] for COPY_STRUCT_BYVAL, as it clobbers CPSR..
Dec 20 2018, 10:06 AM
qcolombet added a comment to D55867: [RegisterCoalescer] dst register's live interval needs to be updated when merging a src register in ToBeUpdated set.

Looks mostly good to me.

Dec 20 2018, 10:03 AM
qcolombet added a comment to D55872: RegBankSelect: Fix copy insertion point for terminators.

Hi Matt,

Dec 20 2018, 9:58 AM
qcolombet added a comment to D55911: GlobalISel: Correct example PartialMapping table.

Hi Matt,

Dec 20 2018, 9:46 AM

Dec 7 2018

qcolombet added inline comments to D55403: [PHIElimination] Allow breaking loop backedges in certain cases..
Dec 7 2018, 8:59 AM
qcolombet added a comment to D55301: RegAlloc: Allow targets to split register allocation.

I'm not sure I follow this. These aren't spilled with ordinary copies

Dec 7 2018, 8:52 AM

Dec 6 2018

qcolombet added a comment to D55301: RegAlloc: Allow targets to split register allocation.

Hi Matt,

Dec 6 2018, 9:15 AM

Nov 29 2018

qcolombet added a comment to D54899: [LoopStrengthReduce] ComplexityLimit as an option.

[...]
With this form, the first three loads can be base + immediate offset, while the last can perform the post increment to create %base.iv.next. What do you think?

Nov 29 2018, 8:43 AM

Nov 28 2018

qcolombet accepted D54899: [LoopStrengthReduce] ComplexityLimit as an option.

LGTM with @gilr's comments addressed.

Nov 28 2018, 3:02 PM
qcolombet added a comment to D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.

I meant we should handle G_BUILD_VECTOR to avoid regressions as we are able to legalize the same function with G_MERGE_VALUES.

Nov 28 2018, 2:58 PM

Nov 27 2018

qcolombet added a comment to D53629: [GlobalISel] Restrict G_MERGE_VALUES capability and replace with new opcodes.

Hi Volkan,

Nov 27 2018, 8:46 AM

Nov 14 2018

qcolombet accepted D54218: [MachineScheduler] Bias physical register immediate assignments.

LGTM

Nov 14 2018, 9:14 AM

Nov 13 2018

qcolombet added a comment to D54218: [MachineScheduler] Bias physical register immediate assignments.

I see a few checks that weaken the check (e.g., -next replaced by regular checks).
Is this expected and why?

Nov 13 2018, 11:53 AM

Nov 8 2018

qcolombet added a comment to D54218: [MachineScheduler] Bias physical register immediate assignments.

Looks reasonable to me.
Could of comments/nitpicks inlined.

Nov 8 2018, 9:12 AM

Nov 7 2018

qcolombet accepted D51861: [LSR] Combine unfolded offset into invariant register.

Hi Gil,

Nov 7 2018, 10:42 AM

Nov 5 2018

qcolombet added inline comments to D51861: [LSR] Combine unfolded offset into invariant register.
Nov 5 2018, 8:55 AM

Nov 2 2018

qcolombet added a comment to D51861: [LSR] Combine unfolded offset into invariant register.

Since testing for optimized or illegal code in these cases isn't an option in these cases, such a LIT could check that the incorrect code is not generated, that the incorrect formula is not generated, or that the correct formula is generated - which all seem fragile and overfitted to this specific bug, so I'm reluctant to add any of them (especially since the bug is covered by 4 runtime tests). What do you think?

Nov 2 2018, 10:08 AM
qcolombet added a comment to D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.

I think the issue of sequences becoming illegal because of things like copies being introduced is still a deal breaker for that, and it would also introduce a code motion barrier since we can't select sequences spanning blocks. E.g. it wouldn't be safe to move the insn in the predecessor due to other uses.

Nov 2 2018, 10:05 AM

Nov 1 2018

qcolombet added a comment to D51861: [LSR] Combine unfolded offset into invariant register.

The failing runtime tests do (a) and (b) but not (c).

Nov 1 2018, 10:11 AM

Oct 31 2018

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

I'm not sure how this is any better. If we had an opcode G_BUILD_VECTOR_TRUNC, it would have a superset of the functionality of the G_BUILD_VECTOR. You'd still have this powerful opcode, except now you have potential code duplication in handling the two, as well as a hindered G_BUILD_VECTOR.

Unless you're say that G_BUILD_VECTOR _TRUNC can *only* deal with oversize scalars, in which case it's an odd opcode for a corner case (and I would suggest here that we just deal with the inconvenient types, but I'm not married to the idea).

Oct 31 2018, 3:47 PM

Oct 30 2018

qcolombet committed rL345644: [InstCombine] Teach the move free before null test opti how to deal with noop….
[InstCombine] Teach the move free before null test opti how to deal with noop…
Oct 30 2018, 1:53 PM
qcolombet closed D53356: [InstCombine] Teach the move free before null test opti how to deal with noop casts.
Oct 30 2018, 1:53 PM
qcolombet added a comment to D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.

Thanks @aditya_nandakumar for the context.

Oct 30 2018, 11:48 AM
qcolombet added a comment to D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes.
This allows implicit truncation.
Oct 30 2018, 10:29 AM
qcolombet added a comment to D51861: [LSR] Combine unfolded offset into invariant register.

Bug fix: ScalarEvolution::getAddExpr() may modify the vector of SCEVs it is given as argument, causing 2nd formula to be created with only part of the SCEVs in Ops.
This patch fixes this by using a temporary copy of Ops for the 1st formula.

Oct 30 2018, 10:27 AM

Oct 29 2018

qcolombet updated the diff for D53356: [InstCombine] Teach the move free before null test opti how to deal with noop casts.
  • Use m_CombineOr instead of repeating the pattern to match
Oct 29 2018, 9:49 AM
qcolombet added a comment to D53356: [InstCombine] Teach the move free before null test opti how to deal with noop casts.

Hi Sanjay,

Oct 29 2018, 9:43 AM

Oct 24 2018

qcolombet added a comment to D53356: [InstCombine] Teach the move free before null test opti how to deal with noop casts.

If it doesn't look too hard, moving it over to SimplifyCFG would be great. That way, we can be sure that the empty block removal always happens without interference from some other pass.

Oct 24 2018, 2:40 PM
qcolombet added a comment to D53356: [InstCombine] Teach the move free before null test opti how to deal with noop casts.

Hi Sanjay,

Oct 24 2018, 10:55 AM

Oct 23 2018

qcolombet added a comment to D53356: [InstCombine] Teach the move free before null test opti how to deal with noop casts.

Ping

Oct 23 2018, 1:37 PM
qcolombet added a comment to D51861: [LSR] Combine unfolded offset into invariant register.

Thanks for the detailed numbers!

Oct 23 2018, 1:35 PM

Oct 19 2018

qcolombet added a comment to D33935: Allow rematerialization of ARM Thumb MOVi8 instruction in some contexts.

However, I would vote for that to be introduced when the problem actually arises. It isn't so obvious to me that many more generalizations of rematerializability will be necessary and the path from enum to bitfield is easy.

What do you think?

Oct 19 2018, 11:01 AM

Oct 17 2018

qcolombet added a comment to D33935: Allow rematerialization of ARM Thumb MOVi8 instruction in some contexts.

Sorry for the delay, I haven’t had time to review the patch but I have one high level comment.
I am not sure the enum approach is desirable as it basically hard code the kind of constraints we can report and will grow very quickly if we want to extend it.
For instance, let say that on top of physreg defs we want to report virtual reg defs, now we would need “yes”, “no”, “yes_phys”, “yes_virt”, “yes_phys_virt” and the list grows with the cross product of everything we may want/need to report.

Oct 17 2018, 1:50 PM

Oct 16 2018

qcolombet created D53356: [InstCombine] Teach the move free before null test opti how to deal with noop casts.
Oct 16 2018, 9:56 PM

Oct 9 2018

qcolombet accepted D35481: Fix documentation of MachineInstr::getNumOperands.

LGTM

Oct 9 2018, 8:52 AM

Oct 8 2018

qcolombet accepted D51861: [LSR] Combine unfolded offset into invariant register.

One question, what is the impact on compile time?
Put differently are the heuristics to prune the search space able to still reduce the space to something manageable?

Oct 8 2018, 3:27 PM

Sep 17 2018

qcolombet added a comment to D52061: [RegisterCoalescer] Only look at main ranges in valuesIdentical/followCopyChain.

But I still believe that trying to spot the identical case in subranges is pointless because we have already proved that the values are identical in main ranges. It is impossible for the values to be not identical in subranges; it's just a bit tricky to fix up the code to demonstrate that.

Sep 17 2018, 9:30 AM
qcolombet added a comment to D52131: [GISel][NFC]: Make MachineIRBuilder fully stateless.

Hi Aditya,

Sep 17 2018, 9:18 AM
qcolombet added a comment to D52127: [MF][MBB]: Add the ability to register callbacks for removal and insertion of MachineInstrs.

Hi Aditya,

Sep 17 2018, 9:12 AM
qcolombet added a comment to D51849: [RegisterCoalescer] Avoid "Use not jointly dominated by defs" in removePartialRedundancy.

I finally tracked down why I couldn't get a mir test before: Compiling a ll file as usual, arriving at RegisterCoalescer, the numbering of the MBBs was not quite in order. Using -stop-before names the MBBs in the mir according to those out-of-order numbers, but reading the mir in again with -run-pass fails to preserve it. Instead it renumbers the MBBs in order. RegisterCoalescing processes MBBs with the same loop depth and critical edgeness and connectedness in numbering order, so the pass behaved differently.

Sep 17 2018, 9:11 AM
qcolombet added a comment to D52052: [RegAllocGreedy] avoid using physreg candidates that cannot be correctly spilled.

Could you elaborate on the exact constraints?

Sep 17 2018, 9:05 AM · Restricted Project

Sep 14 2018

qcolombet added a comment to D52061: [RegisterCoalescer] Only look at main ranges in valuesIdentical/followCopyChain.

The fix here is to only detect an "identical erase" in the main range.

Sep 14 2018, 9:25 AM
qcolombet added a comment to D52052: [RegAllocGreedy] avoid using physreg candidates that cannot be correctly spilled.

Could you elaborate on that part?

But it is not possible since can generate incorrect code in terms of exec mask.

Sep 14 2018, 9:23 AM · Restricted Project

Sep 13 2018

qcolombet added a comment to D51953: [GlobalISel] Add a new IR canonicalization pass.

FWIW, I am still not convince this is a good thing to match SDISel behavior at O0, but this is the least bad solution of doing that IMHO.

Sep 13 2018, 10:12 AM
qcolombet added a comment to D52010: RegAllocFast: Rewrite and improve.

Hi Matthias,

Sep 13 2018, 9:52 AM
qcolombet added inline comments to D51953: [GlobalISel] Add a new IR canonicalization pass.
Sep 13 2018, 9:46 AM
qcolombet accepted D51953: [GlobalISel] Add a new IR canonicalization pass.

Hi Amara,

Sep 13 2018, 9:46 AM

Sep 11 2018

qcolombet added a comment to D51953: [GlobalISel] Add a new IR canonicalization pass.

Hi Amara,

Sep 11 2018, 3:46 PM

Sep 10 2018

qcolombet added a comment to D51849: [RegisterCoalescer] Avoid "Use not jointly dominated by defs" in removePartialRedundancy.

Unfortunately I can only reproduce this in an ll test. Turning it into a mir test makes the problem go away.

Sep 10 2018, 8:40 AM
qcolombet added a comment to D51848: [RegisterCoalescer] Fixup "Fixed inconsistent followCopyChain with subreg".

Hi Tim,

Sep 10 2018, 8:38 AM