Page MenuHomePhabricator

qcolombet (Quentin Colombet)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Wed, Sep 23

qcolombet added a comment to rG6f6d389da5c3: [SplitKit] Only copy live lanes.

Any progress on a fix?

Wed, Sep 23, 4:11 PM
qcolombet accepted D82258: [RegisterCoalescer] Fix IMPLICIT_DEF init removal for a register on joining.

The IMPLICIT_DEF is erased only if there some "incoming" value from "other reg".

Wed, Sep 23, 10:12 AM · Restricted Project

Tue, Sep 22

qcolombet added a comment to D82258: [RegisterCoalescer] Fix IMPLICIT_DEF init removal for a register on joining.

Why would we need to keep this impdef?

Tue, Sep 22, 10:55 AM · Restricted Project
qcolombet added inline comments to rG6f6d389da5c3: [SplitKit] Only copy live lanes.
Tue, Sep 22, 10:46 AM

Mon, Sep 21

qcolombet added a comment to rG6f6d389da5c3: [SplitKit] Only copy live lanes.

Note: A possible fix could be:

diff --git a/llvm/lib/CodeGen/SplitKit.cpp b/llvm/lib/CodeGen/SplitKit.cpp
index 4029c855c910..a1029bcbc71c 100644
--- a/llvm/lib/CodeGen/SplitKit.cpp
+++ b/llvm/lib/CodeGen/SplitKit.cpp
@@ -655,13 +655,20 @@ VNInfo *SplitEditor::defFromParent(unsigned RegIdx,
         if (S.liveAt(UseIdx))
           LaneMask |= S.LaneMask;
       }
-      assert(LaneMask.any() && "Interval has no live subranges");
     } else {
       LaneMask = LaneBitmask::getAll();
     }
Mon, Sep 21, 4:37 PM
qcolombet added inline comments to rG6f6d389da5c3: [SplitKit] Only copy live lanes.
Mon, Sep 21, 4:23 PM
qcolombet accepted D87939: [PeepholeOptimizer] Enhance the redundant COPY elimination..

It does improve the original by considering the COPY from the same subreg.

Mon, Sep 21, 12:12 PM · Restricted Project
qcolombet added a comment to D87939: [PeepholeOptimizer] Enhance the redundant COPY elimination..

The comment seems outdated, if my understanding is right, and even the original code cannot perform that change since, once the 2nd COPY with same source is found in L1407, the check @ L1419 just skips that earlier as the 1st COPY has no subreg and the 2nd COPY has sub1.

Mon, Sep 21, 11:55 AM · Restricted Project
qcolombet added a comment to D87342: Allow targets to augment computeKnownBits with their analysis using TargetTransformInfo.

Ping!

Mon, Sep 21, 10:58 AM · Restricted Project
qcolombet requested changes to D87939: [PeepholeOptimizer] Enhance the redundant COPY elimination..

I must be missing something, but it feels to me that this patch is actually making the situation worse.

Mon, Sep 21, 10:58 AM · Restricted Project

Fri, Sep 18

qcolombet accepted D86925: [MachineSink] add one more profitable pattern for sinking.

Weird, I added case test/CodeGen/PowerPC/sink-down-more-instructions-1.mir. Could you do another check?

Fri, Sep 18, 10:21 AM · Restricted Project

Thu, Sep 17

qcolombet committed rG99e865b618f3: [TargetRegisterInfo] Add a couple of target hooks for the greedy register… (authored by qcolombet).
[TargetRegisterInfo] Add a couple of target hooks for the greedy register…
Thu, Sep 17, 3:25 PM
qcolombet added a comment to D86925: [MachineSink] add one more profitable pattern for sinking.

1: add one more mir test case

Thu, Sep 17, 9:47 AM · Restricted Project

Wed, Sep 16

qcolombet accepted D87760: CodeGen: Move split block utility to MachineBasicBlock.

LGTM

Wed, Sep 16, 11:27 AM · Restricted Project
qcolombet added a comment to D86925: [MachineSink] add one more profitable pattern for sinking.

As expected, the improvement for the benchmark is gone now. : (

Wed, Sep 16, 11:18 AM · Restricted Project

Tue, Sep 15

qcolombet updated the diff for D87342: Allow targets to augment computeKnownBits with their analysis using TargetTransformInfo.
  • Fix a few call sites where I was passing TTI instead of the boolean for UseInstrInfo (it doesn't help that the compiler didn't warm on these)
Tue, Sep 15, 4:31 PM · Restricted Project
qcolombet updated the diff for D87342: Allow targets to augment computeKnownBits with their analysis using TargetTransformInfo.
  • Remove const_cast that was a left over from the proof of concept
Tue, Sep 15, 2:24 PM · Restricted Project
qcolombet added a comment to D87342: Allow targets to augment computeKnownBits with their analysis using TargetTransformInfo.

Gentle ping @nikic.

Tue, Sep 15, 11:16 AM · Restricted Project
qcolombet added a comment to D86925: [MachineSink] add one more profitable pattern for sinking.

The real world case I met is not a loop invariant instruction, it has an operand which is a user of PHI in loop header.

Tue, Sep 15, 11:15 AM · Restricted Project

Mon, Sep 14

qcolombet committed rGb3afad046301: [GlobalISel] Add a `X, Y = G_UNMERGE(G_ZEXT Z)` -> X = G_ZEXT Z; Y = 0 combine (authored by qcolombet).
[GlobalISel] Add a `X, Y = G_UNMERGE(G_ZEXT Z)` -> X = G_ZEXT Z; Y = 0 combine
Mon, Sep 14, 6:30 PM
qcolombet closed D87427: [GlobalISel] Add a `X, Y = G_UNMERGE(G_ZEXT Z)` -> X = G_ZEXT Z; Y = 0 combine.
Mon, Sep 14, 6:30 PM · Restricted Project
qcolombet committed rGd2321129bda7: [GlobalISel] Add `X,Y<dead> = G_UNMERGE Z` -> X = G_TRUNC Z (authored by qcolombet).
[GlobalISel] Add `X,Y<dead> = G_UNMERGE Z` -> X = G_TRUNC Z
Mon, Sep 14, 5:32 PM
qcolombet closed D87174: [GlobalISel] Add `X,Y<dead> = G_UNMERGE Z` -> X = G_TRUNC Z.
Mon, Sep 14, 5:32 PM · Restricted Project
qcolombet committed rGa36278c2f8b5: [GlobalISel] Add G_UNMERGE(Cst) -> Cst1, Cst2, ... combine (authored by qcolombet).
[GlobalISel] Add G_UNMERGE(Cst) -> Cst1, Cst2, ... combine
Mon, Sep 14, 4:32 PM
qcolombet closed D87166: [GlobalISel] Add G_UNMERGE(Cst) -> Cst1, Cst2, ... combine.
Mon, Sep 14, 4:31 PM · Restricted Project
qcolombet committed rG670c276232ec: [GlobalISel] Add G_UNMERGE_VALUES(G_MERGE_VALUES) combine (authored by qcolombet).
[GlobalISel] Add G_UNMERGE_VALUES(G_MERGE_VALUES) combine
Mon, Sep 14, 3:46 PM
qcolombet closed D87117: [GlobalISel] Add G_UNMERGE_VALUES(G_MERGE_VALUES) combine.
Mon, Sep 14, 3:46 PM · Restricted Project
qcolombet added a comment to D82258: [RegisterCoalescer] Fix IMPLICIT_DEF init removal for a register on joining.

I had a (slightly) closer look and I have a question:

Mon, Sep 14, 3:37 PM · Restricted Project
qcolombet updated the diff for D87427: [GlobalISel] Add a `X, Y = G_UNMERGE(G_ZEXT Z)` -> X = G_ZEXT Z; Y = 0 combine.
  • Use Register default constructor instead of forcing = 0
Mon, Sep 14, 2:46 PM · Restricted Project
qcolombet accepted D52010: RegAllocFast: Rewrite and improve.
Mon, Sep 14, 2:22 PM · Restricted Project
qcolombet added a comment to D86925: [MachineSink] add one more profitable pattern for sinking.

The update with the RegClassWeight is only partially done IMHO.
Each weight should be added/removed from the related register pressure sets. You can take a look at MachineLICM.cpp to see how to use these.

Mon, Sep 14, 12:21 PM · Restricted Project
qcolombet abandoned D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Abandon in favor of D87342.

Mon, Sep 14, 11:48 AM · Restricted Project
qcolombet updated the diff for D87342: Allow targets to augment computeKnownBits with their analysis using TargetTransformInfo.

Clean up the proof of concept:

  • Add the proper includes
  • Fixup clang-format acting up
Mon, Sep 14, 11:47 AM · Restricted Project
qcolombet added a comment to D87117: [GlobalISel] Add G_UNMERGE_VALUES(G_MERGE_VALUES) combine.

Ping! @arsenm

Mon, Sep 14, 9:57 AM · Restricted Project

Thu, Sep 10

qcolombet added a comment to D82709: [MachineLICM] [PowerPC] hoisting rematerializable cheap instructions based on register pressure..

Sorry for the late reply I missed your update.

Thu, Sep 10, 11:41 AM · Restricted Project
qcolombet updated the diff for D87174: [GlobalISel] Add `X,Y<dead> = G_UNMERGE Z` -> X = G_TRUNC Z.
  • Use auto for MachineIRBuilder instead of use the register directly
Thu, Sep 10, 10:12 AM · Restricted Project

Wed, Sep 9

qcolombet updated the diff for D87427: [GlobalISel] Add a `X, Y = G_UNMERGE(G_ZEXT Z)` -> X = G_ZEXT Z; Y = 0 combine.
  • Fix typo in comment (G_EXT -> G_ZEXT)
Wed, Sep 9, 6:15 PM · Restricted Project
qcolombet requested review of D87427: [GlobalISel] Add a `X, Y = G_UNMERGE(G_ZEXT Z)` -> X = G_ZEXT Z; Y = 0 combine.
Wed, Sep 9, 6:10 PM · Restricted Project
qcolombet added a comment to D87297: [GlobalISel] Add bailout thresholds to CSEMIRBuilder::dominates() and the localizer..

Hi Amara,

Wed, Sep 9, 12:20 PM · Restricted Project

Tue, Sep 8

qcolombet added inline comments to D87342: Allow targets to augment computeKnownBits with their analysis using TargetTransformInfo.
Tue, Sep 8, 9:05 PM · Restricted Project
qcolombet updated the diff for D87342: Allow targets to augment computeKnownBits with their analysis using TargetTransformInfo.
  • Use TargetTransformInfoImplCRTPBase instead of BasicTTIImpl to avoid requiring the CodeGen library for the unit tests
Tue, Sep 8, 9:05 PM · Restricted Project
qcolombet added a comment to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

I've made a proof of concept in D87342 on how we could have some pieces of computeKnownBits be target dependent.

Tue, Sep 8, 8:39 PM · Restricted Project
qcolombet added a comment to D87342: Allow targets to augment computeKnownBits with their analysis using TargetTransformInfo.

The bulk of the changes are plumbing TTI into the computeKnownBits APIs. On top of that, clang-format did its magic and there are a lot of changes.

Tue, Sep 8, 8:37 PM · Restricted Project
qcolombet requested review of D87342: Allow targets to augment computeKnownBits with their analysis using TargetTransformInfo.
Tue, Sep 8, 8:32 PM · Restricted Project
qcolombet added a comment to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Thanks for the comments.

Tue, Sep 8, 11:08 AM · Restricted Project
qcolombet updated the summary of D87174: [GlobalISel] Add `X,Y<dead> = G_UNMERGE Z` -> X = G_TRUNC Z.
Tue, Sep 8, 10:58 AM · Restricted Project

Fri, Sep 4

qcolombet added inline comments to D87174: [GlobalISel] Add `X,Y<dead> = G_UNMERGE Z` -> X = G_TRUNC Z.
Fri, Sep 4, 6:35 PM · Restricted Project
qcolombet added inline comments to D87174: [GlobalISel] Add `X,Y<dead> = G_UNMERGE Z` -> X = G_TRUNC Z.
Fri, Sep 4, 6:30 PM · Restricted Project
qcolombet requested review of D87174: [GlobalISel] Add `X,Y<dead> = G_UNMERGE Z` -> X = G_TRUNC Z.
Fri, Sep 4, 6:22 PM · Restricted Project
qcolombet added a comment to D87157: [GlobalISel] Add a localizer for copies from physregs and use it in AArch64.

Hi Jessica,

Fri, Sep 4, 5:50 PM · Restricted Project
qcolombet added inline comments to D87166: [GlobalISel] Add G_UNMERGE(Cst) -> Cst1, Cst2, ... combine.
Fri, Sep 4, 3:49 PM · Restricted Project
qcolombet updated the diff for D87166: [GlobalISel] Add G_UNMERGE(Cst) -> Cst1, Cst2, ... combine.
  • Add G_FCONSTANT support
Fri, Sep 4, 3:49 PM · Restricted Project
qcolombet updated the diff for D87166: [GlobalISel] Add G_UNMERGE(Cst) -> Cst1, Cst2, ... combine.
  • Remove getImm(), this is illegal for G_CONSTANT
Fri, Sep 4, 3:30 PM · Restricted Project
qcolombet updated the diff for D87166: [GlobalISel] Add G_UNMERGE(Cst) -> Cst1, Cst2, ... combine.
  • Update AMDGPU tests
Fri, Sep 4, 3:25 PM · Restricted Project
qcolombet requested review of D87166: [GlobalISel] Add G_UNMERGE(Cst) -> Cst1, Cst2, ... combine.
Fri, Sep 4, 2:31 PM · Restricted Project
qcolombet updated the diff for D87117: [GlobalISel] Add G_UNMERGE_VALUES(G_MERGE_VALUES) combine.
  • Use setIntrAndDebugLoc
Fri, Sep 4, 10:32 AM · Restricted Project
qcolombet added inline comments to D87117: [GlobalISel] Add G_UNMERGE_VALUES(G_MERGE_VALUES) combine.
Fri, Sep 4, 10:29 AM · Restricted Project

Thu, Sep 3

qcolombet requested review of D87117: [GlobalISel] Add G_UNMERGE_VALUES(G_MERGE_VALUES) combine.
Thu, Sep 3, 4:16 PM · Restricted Project

Wed, Sep 2

qcolombet requested changes to D86925: [MachineSink] add one more profitable pattern for sinking.
Wed, Sep 2, 12:18 PM · Restricted Project
qcolombet accepted D86888: RegAllocFast: Make self loop live-out heuristic more aggressive.

LGTM with one caveat, do we need the new getOneDefInstr?

Wed, Sep 2, 12:09 PM · Restricted Project
qcolombet accepted D86939: [LSR] Canonicalize a formula before insert it into the list.

LGTM, nitpicks below.

Wed, Sep 2, 12:01 PM · Restricted Project

Tue, Sep 1

qcolombet added a comment to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Let me know if the current compile time impact is acceptable or if I need to come up with a different strategy.

Tue, Sep 1, 9:39 AM · Restricted Project

Wed, Aug 26

qcolombet added a comment to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

One thing we could do to help compile time is maybe add more caching to the Query API.
Right now it only caches llvm.assume AFAICT, whereas we could cache known bits for values. That's what we did in the Machine IR variant of computeKnownBits.

Wed, Aug 26, 11:59 AM · Restricted Project
qcolombet added a comment to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Does this (and follow-up patches) improve any public/private benchmarks? Any data?

Wed, Aug 26, 11:41 AM · Restricted Project
qcolombet updated the diff for D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.
  • Add KnownBits::sextOrTrunc
Wed, Aug 26, 10:29 AM · Restricted Project
qcolombet added a comment to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Thanks for the updated numbers!

Wed, Aug 26, 10:22 AM · Restricted Project

Aug 25 2020

qcolombet added a comment to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

I've made the suggested changes, how do I kick a new compile time measurement?

Aug 25 2020, 10:59 AM · Restricted Project

Aug 24 2020

qcolombet added inline comments to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.
Aug 24 2020, 10:29 AM · Restricted Project
qcolombet updated the diff for D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Use AddrKnownBits.isUnknown instead of having a dedicated bool variable.

Aug 24 2020, 10:24 AM · Restricted Project
qcolombet added a comment to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Possibly adding the early exit on unknown bits will help.

Aug 24 2020, 10:14 AM · Restricted Project
qcolombet added a comment to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Thanks for the review.

Aug 24 2020, 10:08 AM · Restricted Project

Aug 21 2020

qcolombet added inline comments to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.
Aug 21 2020, 9:03 PM · Restricted Project
qcolombet updated the diff for D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.
  • Rework the logic so that TrailZ and AddrKnownBits share most of the logic
  • Fix the handling of structure accesses in the process.
Aug 21 2020, 8:53 PM · Restricted Project
qcolombet added inline comments to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.
Aug 21 2020, 6:34 PM · Restricted Project
qcolombet updated the diff for D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Add scalable vector support (well really abort on scalable vector types!)

Aug 21 2020, 1:19 PM · Restricted Project
qcolombet added a comment to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Let me see if I can massage the IR to work around this issue.

Aug 21 2020, 12:56 PM · Restricted Project
qcolombet added a comment to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Note: I also have one failure with that patch: Analysis/ScalarEvolution/scalable-vector.ll
warning: Compiler has made implicit assumption that TypeSize is not scalable. This may or may not lead to broken code.

Aug 21 2020, 12:50 PM · Restricted Project
qcolombet added a comment to D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Can your add also IR testcase from that PR? (To show that Instcombine can fold it)

Aug 21 2020, 12:48 PM · Restricted Project
qcolombet updated the diff for D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.

Use GEPOperator instead of GEPInstr in cast. GEP can come form constant expression as well.

Aug 21 2020, 12:44 PM · Restricted Project
qcolombet requested review of D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor.
Aug 21 2020, 12:36 PM · Restricted Project
qcolombet added a comment to D82258: [RegisterCoalescer] Fix IMPLICIT_DEF init removal for a register on joining.

Honestly, I have no idea one way or another. I'm leaning toward @arsenm's theory that the implicit def may be needed in case the full register is never fully covered.
But I would need to dig deeper into this.

Aug 21 2020, 12:13 PM · Restricted Project
qcolombet added a comment to D82709: [MachineLICM] [PowerPC] hoisting rematerializable cheap instructions based on register pressure..

I am a bit confused as by what is expected by RA in this case.

Aug 21 2020, 11:59 AM · Restricted Project
qcolombet accepted D82763: MIR: Infer not-SSA for subregister defs.

LGTM

Aug 21 2020, 11:47 AM · Restricted Project

Jul 15 2020

qcolombet committed rG294be6b5d32e: [CalcSpillWeights] Propagate the fact that a live-interval is not spillable (authored by qcolombet).
[CalcSpillWeights] Propagate the fact that a live-interval is not spillable
Jul 15 2020, 5:58 PM

Jul 13 2020

qcolombet added inline comments to rG6c67ee0f5832: [MC] Fix PR45805: infinite recursion in assembler.
Jul 13 2020, 2:39 PM
qcolombet committed rG427bda4e9b37: [MC/AsmParser] layout-interdependency.s depends on having a proper triple (authored by qcolombet).
[MC/AsmParser] layout-interdependency.s depends on having a proper triple
Jul 13 2020, 2:39 PM
qcolombet added inline comments to rG6c67ee0f5832: [MC] Fix PR45805: infinite recursion in assembler.
Jul 13 2020, 1:46 PM
qcolombet added inline comments to rG6c67ee0f5832: [MC] Fix PR45805: infinite recursion in assembler.
Jul 13 2020, 1:34 PM
qcolombet added inline comments to rG6c67ee0f5832: [MC] Fix PR45805: infinite recursion in assembler.
Jul 13 2020, 9:10 AM

Jul 10 2020

qcolombet added inline comments to rG6c67ee0f5832: [MC] Fix PR45805: infinite recursion in assembler.
Jul 10 2020, 4:54 PM

Jun 3 2020

qcolombet committed rGccb3c8e86134: [RegisterCoalescer] Update empty subranges when rematerializing (authored by qcolombet).
[RegisterCoalescer] Update empty subranges when rematerializing
Jun 3 2020, 5:40 PM

May 21 2020

qcolombet added a comment to D75098: Add TCOPY, a terminator form of the COPY instr.

Regardless of what happens to INLINEASM_BR, I think we still need a terminator copy

May 21 2020, 12:59 PM · Restricted Project

May 5 2020

qcolombet added a comment to D75098: Add TCOPY, a terminator form of the COPY instr.

Thanks Bill for the detailed explanations. A couple more questions.

May 5 2020, 4:45 PM · Restricted Project

May 4 2020

Herald added a project to D76135: [MachineLICM] Don't treat cross copies as cheap: Restricted Project.
May 4 2020, 10:43 AM · Restricted Project
qcolombet accepted D63746: MachineFrameInfo: Make StackSize Optional..
May 4 2020, 10:43 AM
qcolombet added a comment to D52010: RegAllocFast: Rewrite and improve.

I've scanned through the patch and it looks sensible.

May 4 2020, 10:43 AM · Restricted Project
qcolombet accepted D78248: [RegisterCoalescer] Extend a subrange if needed when filling range gap.
May 4 2020, 10:09 AM · Restricted Project
qcolombet requested changes to D75098: Add TCOPY, a terminator form of the COPY instr.

I don't think introducing the TCOPY opcode is a valid solution.
Let's assume the copy doesn't get coalesced, that means it needs to be executed after the inlineasm branch. And this is not going to happen if it sits in the same basic block right after the jump.

May 4 2020, 10:09 AM · Restricted Project
qcolombet accepted D79301: [LSR] Don't require reuse register under postinc.
May 4 2020, 10:09 AM · Restricted Project

May 1 2020

qcolombet added a reviewer for D78964: [Support] Handle signed padding in decodeLEB128 + add multi-sized tests: dsanders.
May 1 2020, 5:53 PM · Restricted Project