Page MenuHomePhabricator

ctetreau (Christopher Tetreault)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 11 2019, 12:14 PM (59 w, 5 d)

Recent Activity

Tue, Dec 1

ctetreau added inline comments to D91174: [Analysis] Introduce a new InstructionCost class.
Tue, Dec 1, 8:43 AM · Restricted Project
ctetreau added inline comments to D92178: [InstructionCost] Change LoopVectorizationCostModel::getInstructionCost to return InstructionCost.
Tue, Dec 1, 8:39 AM · Restricted Project
ctetreau added a comment to D90342: [POC][LoopVectorizer] Propagate ElementCount to interfaces in preparation for scalable auto-vec. .

It looks like all the split-out patches have been merged. Is there anything in this patch that is still outstanding?

Tue, Dec 1, 4:28 AM · Restricted Project

Mon, Nov 30

ctetreau added a comment to D92177: [NFC][InstructionCost] Refactor LoopVectorizationCostModel::selectVectorizationFactor.

I don't think you needs these pairs. If you check validity of the InstructionCost in the for loop, and break or continue if it's invalid, then you can just use a scalar value as it did before. (I commented as such on D92178)

Mon, Nov 30, 9:58 AM · Restricted Project
ctetreau added inline comments to D92178: [InstructionCost] Change LoopVectorizationCostModel::getInstructionCost to return InstructionCost.
Mon, Nov 30, 9:56 AM · Restricted Project
ctetreau added inline comments to D92177: [NFC][InstructionCost] Refactor LoopVectorizationCostModel::selectVectorizationFactor.
Mon, Nov 30, 7:32 AM · Restricted Project
ctetreau added a comment to D92094: [CostModel]Replace FixedVectorType by VectorType in costgetIntrinsicInstrCost.

I'd prefer if we didn't knowingly create broken paths here

Mon, Nov 30, 7:18 AM · Restricted Project
ctetreau added a comment to D91174: [Analysis] Introduce a new InstructionCost class.

I personally feel very strongly that we should define a total ordering here as is currently being done. @nhaehnle says that this behaves like nan, and that this is bad, and that it runs the risk of users relying on this total ordering. I disagree with all three of these concerns.

Mon, Nov 30, 5:40 AM · Restricted Project

Tue, Nov 24

ctetreau accepted D91174: [Analysis] Introduce a new InstructionCost class.

I'll go ahead an approve the patch for now to clear the "red x" next to my name. Please add the test before merging though.

Tue, Nov 24, 9:02 AM · Restricted Project
ctetreau added a comment to D91174: [Analysis] Introduce a new InstructionCost class.

I don't know if having that red x next to my name causes phabricator to be difficult. If so, I apologize.

Tue, Nov 24, 9:00 AM · Restricted Project
ctetreau added a comment to D91174: [Analysis] Introduce a new InstructionCost class.

I marked it "needs changes" to rescind my acceptance due to Sander's issue. However, I'm fine with this (aside from the test I requested), so feel free to merge it once the test is added and Sander is satisfied.

Tue, Nov 24, 9:00 AM · Restricted Project

Mon, Nov 23

ctetreau added a comment to D91174: [Analysis] Introduce a new InstructionCost class.

I suppose I'm fine with Optional<CostType> for the purpose of future-proofing. Users should be doing arithmetic on InstructionCost anyways, so there shouldn't be a ton of conversions. Maybe we can just provide operator* instead of making the user write *(C.getCost()).

Mon, Nov 23, 8:50 AM · Restricted Project
ctetreau added a comment to D91174: [Analysis] Introduce a new InstructionCost class.

I'm likely to be out of the office most of this week, so once the assert is in, feel free to merge without waiting for my approval (assuming Sander is satisfied)

Mon, Nov 23, 8:47 AM · Restricted Project
ctetreau requested changes to D91174: [Analysis] Introduce a new InstructionCost class.
Mon, Nov 23, 8:46 AM · Restricted Project

Fri, Nov 20

ctetreau added a comment to D91727: [LAA] NFC: Rename [get]MaxSafeRegisterWidth -> [get]MaxSafeVectorWidthInBits.

Please run clang-format, then this looks good to me.

Fri, Nov 20, 3:18 PM · Restricted Project
ctetreau added a comment to D90344: [POC][LoopVectorizer] Allow invariant loads/stores using masked gather/scatter for a scalable VF..

I have no objections to this, but you should probably get some more eyes on it.

Fri, Nov 20, 9:50 AM · Restricted Project
ctetreau accepted D91174: [Analysis] Introduce a new InstructionCost class.

Looks good to me

Fri, Nov 20, 9:46 AM · Restricted Project

Thu, Nov 19

ctetreau added a comment to D91174: [Analysis] Introduce a new InstructionCost class.

This is pretty much there, just a few nits and it will be good to go.

Thu, Nov 19, 4:45 PM · Restricted Project
ctetreau added inline comments to D91727: [LAA] NFC: Rename [get]MaxSafeRegisterWidth -> [get]MaxSafeVectorWidthInBits.
Thu, Nov 19, 4:28 PM · Restricted Project

Tue, Nov 17

ctetreau committed rG792f8e1114af: [SVE] Take constant fold fast path for splatted vscale vectors (authored by ctetreau).
[SVE] Take constant fold fast path for splatted vscale vectors
Tue, Nov 17, 12:46 PM
ctetreau closed D77442: [SVE] Take constant fold fast path for splatted vscale vectors.
Tue, Nov 17, 12:45 PM · Restricted Project

Mon, Nov 16

ctetreau updated the diff for D77442: [SVE] Take constant fold fast path for splatted vscale vectors.

Fix broken test

Mon, Nov 16, 3:09 PM · Restricted Project
ctetreau added a comment to D77442: [SVE] Take constant fold fast path for splatted vscale vectors.

Ping?

Mon, Nov 16, 1:48 PM · Restricted Project
ctetreau updated the diff for D77442: [SVE] Take constant fold fast path for splatted vscale vectors.

Necro patch. address code review issues.

Mon, Nov 16, 1:45 PM · Restricted Project
ctetreau abandoned D91564: [SVE] Take constant fold fast path for splatted vscale vectors.
Mon, Nov 16, 1:45 PM · Restricted Project
ctetreau requested review of D91564: [SVE] Take constant fold fast path for splatted vscale vectors.
Mon, Nov 16, 1:42 PM · Restricted Project

Fri, Nov 13

ctetreau added inline comments to D91174: [Analysis] Introduce a new InstructionCost class.
Fri, Nov 13, 10:20 AM · Restricted Project

Tue, Nov 10

ctetreau added inline comments to D91174: [Analysis] Introduce a new InstructionCost class.
Tue, Nov 10, 10:11 AM · Restricted Project
ctetreau added a comment to D91174: [Analysis] Introduce a new InstructionCost class.

Overall, this seems reasonable to me.

Tue, Nov 10, 10:09 AM · Restricted Project

Mon, Nov 9

ctetreau accepted D90880: [LoopVectorizer] NFC: Return ElementCount from compute[Feasible]MaxVF.

looks good to me

Mon, Nov 9, 9:38 AM · Restricted Project
ctetreau accepted D90879: [LoopVectorizer] NFC: Propagate ElementCount to more interfaces..

looks good to me

Mon, Nov 9, 9:31 AM · Restricted Project

Fri, Nov 6

ctetreau added inline comments to D90761: [docs] Adding a Support Policy.
Fri, Nov 6, 4:00 PM · Restricted Project

Thu, Nov 5

ctetreau added a reviewer for D90344: [POC][LoopVectorizer] Allow invariant loads/stores using masked gather/scatter for a scalable VF.: ctetreau.
Thu, Nov 5, 2:22 PM · Restricted Project
ctetreau added inline comments to D90344: [POC][LoopVectorizer] Allow invariant loads/stores using masked gather/scatter for a scalable VF..
Thu, Nov 5, 2:21 PM · Restricted Project
ctetreau added inline comments to D90342: [POC][LoopVectorizer] Propagate ElementCount to interfaces in preparation for scalable auto-vec. .
Thu, Nov 5, 2:16 PM · Restricted Project
ctetreau accepted D90713: [TypeSize] Extend UnivariateLinearPolyBase with getWithIncrement/Decrement methods.

looks good to me

Thu, Nov 5, 2:09 PM · Restricted Project
ctetreau added inline comments to D90761: [docs] Adding a Support Policy.
Thu, Nov 5, 9:33 AM · Restricted Project

Wed, Nov 4

ctetreau added a comment to D90761: [docs] Adding a Support Policy.

I've suggested some changes, but this seems reasonable to me.

Wed, Nov 4, 11:55 AM · Restricted Project
ctetreau committed rG900ec97bbe32: [UBSan] Cannot negate smallest negative signed integer (authored by ctetreau).
[UBSan] Cannot negate smallest negative signed integer
Wed, Nov 4, 10:08 AM
ctetreau closed D90710: [UBSan] Cannot negate smallest negative signed integer.
Wed, Nov 4, 10:08 AM · Restricted Project

Tue, Nov 3

ctetreau added reviewers for D90710: [UBSan] Cannot negate smallest negative signed integer: paulwalker-arm, kmclaughlin, cameron.mcinally, c-rhodes, efriedma.
Tue, Nov 3, 1:45 PM · Restricted Project
ctetreau requested review of D90710: [UBSan] Cannot negate smallest negative signed integer.
Tue, Nov 3, 12:59 PM · Restricted Project

Oct 30 2020

ctetreau accepted D88982: [NFCI] Add StackOffset class and base classes for ElementCount, TypeSize..

Yeah, I think this is good to go

Oct 30 2020, 12:57 PM · Restricted Project, Restricted Project

Oct 29 2020

ctetreau added a reviewer for D90342: [POC][LoopVectorizer] Propagate ElementCount to interfaces in preparation for scalable auto-vec. : ctetreau.
Oct 29 2020, 1:41 PM · Restricted Project
ctetreau added a comment to D90342: [POC][LoopVectorizer] Propagate ElementCount to interfaces in preparation for scalable auto-vec. .

This seems reasonable to me.

Oct 29 2020, 1:41 PM · Restricted Project

Oct 28 2020

ctetreau added a comment to D88982: [NFCI] Add StackOffset class and base classes for ElementCount, TypeSize..

Looks good to me once you add that assert.

Oct 28 2020, 9:50 AM · Restricted Project, Restricted Project

Oct 27 2020

ctetreau abandoned D89818: [Compile Time] Make it possible to skip redundant debuginst removal.

The other patch was merged.

Oct 27 2020, 11:55 AM · Restricted Project
ctetreau accepted D88928: [Utils] Skip RemoveRedundantDbgInstrs in MergeBlockIntoPredecessor (PR47746).

Looks good to me.

Oct 27 2020, 9:40 AM · Restricted Project

Oct 26 2020

ctetreau added inline comments to D88982: [NFCI] Add StackOffset class and base classes for ElementCount, TypeSize..
Oct 26 2020, 4:17 PM · Restricted Project, Restricted Project
ctetreau added inline comments to D88982: [NFCI] Add StackOffset class and base classes for ElementCount, TypeSize..
Oct 26 2020, 1:00 PM · Restricted Project, Restricted Project
ctetreau added inline comments to D88982: [NFCI] Add StackOffset class and base classes for ElementCount, TypeSize..
Oct 26 2020, 12:04 PM · Restricted Project, Restricted Project

Oct 22 2020

ctetreau added inline comments to D88982: [NFCI] Add StackOffset class and base classes for ElementCount, TypeSize..
Oct 22 2020, 2:46 PM · Restricted Project, Restricted Project

Oct 21 2020

ctetreau added a comment to D88928: [Utils] Skip RemoveRedundantDbgInstrs in MergeBlockIntoPredecessor (PR47746).
In D88928#2343175, @vsk wrote:

I was hoping for another +1 after Jeremy lgtm'd this change to be safe. Seeing as this appears to be time-sensitive, if there aren't any concerns/objections I'll land this by end of day tomorrow (PST).

Oct 21 2020, 1:54 PM · Restricted Project
ctetreau added inline comments to D88982: [NFCI] Add StackOffset class and base classes for ElementCount, TypeSize..
Oct 21 2020, 10:16 AM · Restricted Project, Restricted Project
ctetreau committed rG26459e6d8eeb: Fix "Unknown arguments specified" to if in lldb (authored by ctetreau).
Fix "Unknown arguments specified" to if in lldb
Oct 21 2020, 7:25 AM
ctetreau closed D89807: Fix "Unknown arguments specified" to if in lldb.
Oct 21 2020, 7:25 AM · Restricted Project

Oct 20 2020

ctetreau accepted D89531: [SVE] Remove reliance on TypeSize comparison operators in unit tests.

looks good to me

Oct 20 2020, 5:11 PM · Restricted Project
ctetreau added a comment to D89818: [Compile Time] Make it possible to skip redundant debuginst removal.

I think your patch is reasonable. Mine is more conservative, yours more aggressive. Both solve the problem. If this patch is merged, yours should be abandoned.

Oct 20 2020, 3:51 PM · Restricted Project
ctetreau added a comment to D88928: [Utils] Skip RemoveRedundantDbgInstrs in MergeBlockIntoPredecessor (PR47746).

I don't know if this patch as-is exhaustively removes redundant debug instructions in all cases it did before, or if we care, so I can't approve it, but I am OK with this approach.

Oct 20 2020, 3:50 PM · Restricted Project
ctetreau added a comment to D89813: [CMake][NFC] Limit the use of uppercase_CMAKE_BUILD_TYPE.

I would assume so. I'll take a look.

Oct 20 2020, 3:45 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
ctetreau added a comment to D89807: Fix "Unknown arguments specified" to if in lldb.

OK, for now, I've restored the use of uppercase_CMAKE_BUILD_TYPE. It looks like this does get set, and it's just the empty string on my machine because windows.

Oct 20 2020, 3:25 PM · Restricted Project
ctetreau updated the diff for D89807: Fix "Unknown arguments specified" to if in lldb.

restore use of uppercase_CMAKE_BUILD_TYPE

Oct 20 2020, 3:22 PM · Restricted Project
ctetreau committed rG2eac8ce820e6: Get the address space within getVectorPtrTy (authored by ctetreau).
Get the address space within getVectorPtrTy
Oct 20 2020, 2:42 PM
ctetreau closed D89745: Get the address space within getVectorPtrTy.
Oct 20 2020, 2:42 PM · Restricted Project
ctetreau updated the diff for D89818: [Compile Time] Make it possible to skip redundant debuginst removal.

address code review issues

Oct 20 2020, 2:05 PM · Restricted Project
ctetreau added a comment to D89818: [Compile Time] Make it possible to skip redundant debuginst removal.

Is this an NFC patch? If not, can you please add a test case?

Oct 20 2020, 2:03 PM · Restricted Project
ctetreau added a comment to D88928: [Utils] Skip RemoveRedundantDbgInstrs in MergeBlockIntoPredecessor (PR47746).

I pushed D89818, which is based on the patch that I submitted to my downstream to fix this issue.

Oct 20 2020, 1:01 PM · Restricted Project
ctetreau added a comment to D89818: [Compile Time] Make it possible to skip redundant debuginst removal.

This patch is basically the same thing we ended up doing in our downstream to resolve the issue. We're seeing it in LoopUnroll, so that's where we are skipping it.

Oct 20 2020, 12:59 PM · Restricted Project
ctetreau added reviewers for D89818: [Compile Time] Make it possible to skip redundant debuginst removal: fhahn, bjope, aprantl, jmorse, efriedma.
Oct 20 2020, 12:57 PM · Restricted Project
ctetreau requested review of D89818: [Compile Time] Make it possible to skip redundant debuginst removal.
Oct 20 2020, 12:55 PM · Restricted Project
ctetreau added a comment to D89813: [CMake][NFC] Limit the use of uppercase_CMAKE_BUILD_TYPE.

I would assume so. I'll take a look.

Oct 20 2020, 12:37 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
ctetreau abandoned D89813: [CMake][NFC] Limit the use of uppercase_CMAKE_BUILD_TYPE.
Oct 20 2020, 12:15 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
ctetreau added a comment to D89813: [CMake][NFC] Limit the use of uppercase_CMAKE_BUILD_TYPE.

I quite like this. We should strive to use the same name as CMake uses for its build types, i.e. Debug, Release, etc.

If I configure CMake with -DCMAKE_BUILD_TYPE=DEBUG, is CMAKE_BUILD_TYPE normalized back to Debug by CMake, or is it defined to DEBUG? If the latter, then I suggest we might want to add a temporary error message when the CMAKE_BUILD_TYPE is defined to something all-uppercase. This would help catch cases where this patch will be a change in behavior. That would be a nice place to tell users to use the right build type names.

Oct 20 2020, 12:15 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
ctetreau planned changes to D89807: Fix "Unknown arguments specified" to if in lldb.

After experimenting a bit, it seems that if you pass -DCMAKE_BUILD_TYPE=RELEASE, then the value of CMAKE_BUILD_TYPE will also be uppercase. This might represent a "good reason" to use the uppercase_CMAKE_BUILD_TYPE version. I need to think on this a bit, but I may end up just making sure that this variable is defined.

Oct 20 2020, 11:59 AM · Restricted Project
ctetreau requested review of D89813: [CMake][NFC] Limit the use of uppercase_CMAKE_BUILD_TYPE.
Oct 20 2020, 11:29 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
ctetreau added a comment to D89807: Fix "Unknown arguments specified" to if in lldb.

What's the motivation for this?

Oct 20 2020, 11:04 AM · Restricted Project
ctetreau added a reviewer for D89807: Fix "Unknown arguments specified" to if in lldb: efriedma.
Oct 20 2020, 10:11 AM · Restricted Project
ctetreau added reviewers for D89807: Fix "Unknown arguments specified" to if in lldb: JDevlieghere, phosek, teemperor.
Oct 20 2020, 10:10 AM · Restricted Project
ctetreau requested review of D89807: Fix "Unknown arguments specified" to if in lldb.
Oct 20 2020, 10:07 AM · Restricted Project

Oct 19 2020

ctetreau added reviewers for D89745: Get the address space within getVectorPtrTy: Meinersbur, efriedma, gchatelet.
Oct 19 2020, 3:48 PM · Restricted Project
ctetreau requested review of D89745: Get the address space within getVectorPtrTy.
Oct 19 2020, 3:14 PM · Restricted Project

Oct 14 2020

ctetreau added a comment to D88982: [NFCI] Add StackOffset class and base classes for ElementCount, TypeSize..

@sdesmalen I'm generally ok with this approach. This patch is pretty complicated, so I haven't had the time to fully digest it and decide if it's actually correct. I plan to try to find time to do this soon, but I'd feel more comfortable if others took a look as well.

Oct 14 2020, 1:35 PM · Restricted Project, Restricted Project

Oct 9 2020

ctetreau accepted D88409: [SVE] Make ElementCount and TypeSize use a new PolySize class.

looks good to me

Oct 9 2020, 3:56 PM · Restricted Project, Restricted Project

Oct 7 2020

ctetreau added inline comments to D88982: [NFCI] Add StackOffset class and base classes for ElementCount, TypeSize..
Oct 7 2020, 2:27 PM · Restricted Project, Restricted Project
ctetreau added a comment to D88982: [NFCI] Add StackOffset class and base classes for ElementCount, TypeSize..

Are you suggesting rename PolyBase or just to remove the term 'polynomial' from the commit message and any of the comments?

Oct 7 2020, 2:06 PM · Restricted Project, Restricted Project
ctetreau added a comment to D88982: [NFCI] Add StackOffset class and base classes for ElementCount, TypeSize..

This isn't really a true polynomial because it doesn't support multiplication of two terms: 2x * 2x = 4x^2

Oct 7 2020, 11:10 AM · Restricted Project, Restricted Project
ctetreau added inline comments to D88409: [SVE] Make ElementCount and TypeSize use a new PolySize class.
Oct 7 2020, 10:09 AM · Restricted Project, Restricted Project
ctetreau added a comment to D88409: [SVE] Make ElementCount and TypeSize use a new PolySize class.

Hi @ctetreau, I decided to remove the default constructors as we only need a one or two line change llvm/include/llvm/IR/Intrinsics.h. This removes the need for asserts that we have a properly constructed object. Does this seem reasonable?

Oct 7 2020, 9:53 AM · Restricted Project, Restricted Project

Oct 5 2020

ctetreau added inline comments to D88741: [SystemZ/z/OS] Add utility class for char set conversion..
Oct 5 2020, 10:20 AM · Restricted Project
ctetreau added inline comments to D88409: [SVE] Make ElementCount and TypeSize use a new PolySize class.
Oct 5 2020, 9:37 AM · Restricted Project, Restricted Project
ctetreau added inline comments to D88409: [SVE] Make ElementCount and TypeSize use a new PolySize class.
Oct 5 2020, 9:25 AM · Restricted Project, Restricted Project

Sep 30 2020

ctetreau added inline comments to D88409: [SVE] Make ElementCount and TypeSize use a new PolySize class.
Sep 30 2020, 6:27 AM · Restricted Project, Restricted Project

Sep 29 2020

ctetreau added inline comments to D88409: [SVE] Make ElementCount and TypeSize use a new PolySize class.
Sep 29 2020, 10:54 AM · Restricted Project, Restricted Project
ctetreau added inline comments to D88409: [SVE] Make ElementCount and TypeSize use a new PolySize class.
Sep 29 2020, 7:56 AM · Restricted Project, Restricted Project
ctetreau added inline comments to D88409: [SVE] Make ElementCount and TypeSize use a new PolySize class.
Sep 29 2020, 6:17 AM · Restricted Project, Restricted Project

Sep 28 2020

ctetreau added inline comments to D88409: [SVE] Make ElementCount and TypeSize use a new PolySize class.
Sep 28 2020, 6:44 AM · Restricted Project, Restricted Project

Sep 24 2020

ctetreau added a comment to D87700: [SVE] Replace / operator in TypeSize/ElementCount with divideCoefficientBy.

I'm fine with the substance of this patch. Once we come to an agreement about what color the bikeshed should be, I think it's good to go.

Sep 24 2020, 11:49 AM · Restricted Project, Restricted Project
ctetreau added a comment to D87700: [SVE] Replace / operator in TypeSize/ElementCount with divideCoefficientBy.

I just wanted to voice my concerns for halve(). If nobody else has an opinion on the subject, then I'll not hold the review up over it.

Sep 24 2020, 11:46 AM · Restricted Project, Restricted Project
ctetreau added a comment to D87700: [SVE] Replace / operator in TypeSize/ElementCount with divideCoefficientBy.

I'm not in love with the halve function. I count 8 places where it's used in real code in this patch, which really isn't that much. I think having it in there will establish a precedent and encourage people to add a million "common case" helpers (why not double? quarter/quadruple? next/prev power of 2?) for constructing different sized ElementCounts.

Sep 24 2020, 9:25 AM · Restricted Project, Restricted Project
ctetreau added a comment to D87700: [SVE] Replace / operator in TypeSize/ElementCount with divideCoefficientBy.

... thoughts on Sander's suggestion of divideCoefficientBy?

Sep 24 2020, 9:07 AM · Restricted Project, Restricted Project