Page MenuHomePhabricator

Ayal (Ayal Zaks)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 12 2015, 1:48 PM (363 w, 4 d)

Recent Activity

Yesterday

Ayal accepted D128831: [VPlan] Remove dead recipes before merging regions..

This minimal improvement is fine, thanks! Adding some optional nits:

Thu, Jun 30, 1:33 PM · Restricted Project, Restricted Project
Ayal accepted D128408: [LV] Remove collectTriviallyDeadInstructions, already handled by VP DCE..

Nice clean-up!
Adding a nit for potential further clean-ups.

Thu, Jun 30, 5:16 AM · Restricted Project, Restricted Project

Wed, Jun 29

Ayal accepted D128755: [VPlan] Make sure optimizeInductions removes wide ind from scalar plan..

This fix is fine, thanks, adding a minor nit.

Wed, Jun 29, 10:58 AM · Restricted Project, Restricted Project
Ayal added inline comments to D128408: [LV] Remove collectTriviallyDeadInstructions, already handled by VP DCE..
Wed, Jun 29, 7:27 AM · Restricted Project, Restricted Project
Ayal added inline comments to D128755: [VPlan] Make sure optimizeInductions removes wide ind from scalar plan..
Wed, Jun 29, 12:59 AM · Restricted Project, Restricted Project

Tue, Jun 28

Ayal added inline comments to D128408: [LV] Remove collectTriviallyDeadInstructions, already handled by VP DCE..
Tue, Jun 28, 1:36 PM · Restricted Project, Restricted Project
Ayal accepted D127966: [LV] Move LoopVersioning creation to LVP::execute..

Add extra check if there are any runtime checks before creating LoopVersioning, move code up.

Great, thanks!
Currently noalias is introduced by emitMemRuntimeChecks() when called from ILV::createVectorizedLoopSkeleton() and from its overriding EpilogueVectorizerMainLoop::createEpilogueVectorizedLoopSkeleton(), but is missed by its overriding EpilogueVectorizerEpilogueLoop::createEpilogueVectorizedLoopSkeleton(). How about placing it in completeLoopSkeleton(), which all three skeleton creators call? It does, after all, seem to belong to ILV* than to LVP?

The follow-up patches move addMetadata to VPTransformState, which requires VPTransformState to have access to LoopVersioning. If we move the code to completeLoopSkeleton, it would have to be updated to return the LoopVersioning. Please let me know if you prefer it that way and I'll updated the patch :)

Tue, Jun 28, 2:19 AM · Restricted Project, Restricted Project

Mon, Jun 27

Ayal accepted D128657: [VPlan] Move setDebugLocFromInst to VPTransformState (NFC)..
Mon, Jun 27, 1:00 PM · Restricted Project, Restricted Project
Ayal accepted D127965: [VPlan] Move recipe implementations to separate file (NFC)..

Address latest comments, thanks!

Nice move! Would it be reasonable to also have a matching VPlanRecipes.h, excluding VPRecipeBase?

I think this would require a bit more surgery, as VPlan.h contains multiple function definitions that require the full definition of different recipes types. This could be resolved by moving them to VPlan.cpp, but I think that would be best done separately. Also, I think in most file we would have to include both headers in any case?

Mon, Jun 27, 9:55 AM · Restricted Project, Restricted Project
Ayal added a comment to D127966: [LV] Move LoopVersioning creation to LVP::execute..

Add extra check if there are any runtime checks before creating LoopVersioning, move code up.

Mon, Jun 27, 9:48 AM · Restricted Project, Restricted Project
Ayal added inline comments to D128033: [LoopVectorize] Uninitialized phi node leads to a crash in SSAUpdater..
Mon, Jun 27, 1:08 AM · Restricted Project, Restricted Project

Sun, Jun 26

Ayal added inline comments to D128408: [LV] Remove collectTriviallyDeadInstructions, already handled by VP DCE..
Sun, Jun 26, 9:57 PM · Restricted Project, Restricted Project
Ayal accepted D127970: [VPlan] Move VPWidenSelectRecipe::execute to VPlanRecipes.cpp (NFC)..

Ship it!

Sun, Jun 26, 2:19 PM · Restricted Project, Restricted Project
Ayal accepted D127968: [VPlan] Move addMetadata to VPTransformState (NFC)..

Ship it!

Sun, Jun 26, 2:16 PM · Restricted Project, Restricted Project
Ayal added a comment to D127966: [LV] Move LoopVersioning creation to LVP::execute..

Curious if current lack of proper noalias metadata of an epilogue vector loop may result in missed optimization or potentially wrong code?

Sun, Jun 26, 2:09 PM · Restricted Project, Restricted Project
Ayal added a comment to D127965: [VPlan] Move recipe implementations to separate file (NFC)..

Nice move! Would it be reasonable to also have a matching VPlanRecipes.h, excluding VPRecipeBase?

Sun, Jun 26, 1:47 PM · Restricted Project, Restricted Project

Mon, Jun 13

Ayal accepted D127580: [VPlan] Remove dead recipes across whole plan..

Looks good to me!

Mon, Jun 13, 8:24 AM · Restricted Project, Restricted Project

Thu, Jun 2

Ayal accepted D126679: [VPlan] Update vector latch terminator edge to exit block after execution..

Ship it!

Thu, Jun 2, 8:30 AM · Restricted Project, Restricted Project
Ayal accepted D126680: [VPlan] Replace BranchOnCount with BranchOnCond if TC <= UF * VF..

This looks good to me, thanks!

Thu, Jun 2, 3:07 AM · Restricted Project, Restricted Project
Ayal accepted D126618: [VPlan] Replace CondBit with BranchOnCond VPInstruction..

This looks good to me, thanks!
Adding minor nits.

Thu, Jun 2, 1:43 AM · Restricted Project, Restricted Project
Ayal added inline comments to D126679: [VPlan] Update vector latch terminator edge to exit block after execution..
Thu, Jun 2, 1:08 AM · Restricted Project, Restricted Project

Wed, Jun 1

Ayal added inline comments to D126618: [VPlan] Replace CondBit with BranchOnCond VPInstruction..
Wed, Jun 1, 1:02 PM · Restricted Project, Restricted Project

May 31 2022

Ayal accepted D123005: [VPlan] Use region for each loop in native path..

This looks good to me, ship it!

May 31 2022, 1:53 PM · Restricted Project, Restricted Project

May 29 2022

Ayal added a comment to D126618: [VPlan] Replace CondBit with BranchOnCond VPInstruction..

Should tests check for new BranchOnCond VPInstruction?

May 29 2022, 11:39 PM · Restricted Project, Restricted Project
Ayal added inline comments to D123005: [VPlan] Use region for each loop in native path..
May 29 2022, 3:11 PM · Restricted Project, Restricted Project

May 22 2022

Ayal accepted D126173: [VPlan] Use Exiting-block instead of Exit-block terminology (NFC)..

Thanks for following up! Minor nits to help keep consistency.

May 22 2022, 2:59 PM · Restricted Project, Restricted Project
Ayal accepted D125029: [VPlan] Exit earlier when trying to widen with scalar VFs..

Thanks for following up!

May 22 2022, 2:39 PM · Restricted Project, Restricted Project

May 21 2022

Ayal added inline comments to D123537: [VPlan] Model first exit values using VPLiveOut..
May 21 2022, 12:49 PM · Restricted Project, Restricted Project
Ayal added a comment to D125029: [VPlan] Exit earlier when trying to widen with scalar VFs..

Thanks for following up!

May 21 2022, 1:17 AM · Restricted Project, Restricted Project

May 20 2022

Ayal accepted D123537: [VPlan] Model first exit values using VPLiveOut..

Looks good to me, thanks!

May 20 2022, 5:30 AM · Restricted Project, Restricted Project
Ayal added inline comments to rG5b00d13c0071: [LV] Fetch vector loop region once and remember it (NFC)..
May 20 2022, 5:14 AM · Restricted Project, Restricted Project

May 19 2022

Ayal added a comment to D123537: [VPlan] Model first exit values using VPLiveOut..

Looks good to me! Adding minor suggestions.

May 19 2022, 11:20 PM · Restricted Project, Restricted Project

May 17 2022

Ayal added inline comments to D123537: [VPlan] Model first exit values using VPLiveOut..
May 17 2022, 1:31 PM · Restricted Project, Restricted Project

May 16 2022

Ayal added a comment to D123537: [VPlan] Model first exit values using VPLiveOut..

Summary deserves updating.

May 16 2022, 2:46 PM · Restricted Project, Restricted Project
Ayal accepted D124936: [VPlan] Move usesScalars/onlyFirstLaneUsed to VPUser..

Looks good! Minor documentation update nits.

May 16 2022, 10:51 AM · Restricted Project, Restricted Project

May 2 2022

Ayal accepted D124718: [VPlan] Do not create VPWidenCall recipes for scalar vector factors..

Looks good to me, adding a couple of nits.

May 2 2022, 4:50 AM · Restricted Project, Restricted Project
Ayal added inline comments to D123537: [VPlan] Model first exit values using VPLiveOut..
May 2 2022, 12:15 AM · Restricted Project, Restricted Project

Apr 28 2022

Ayal added inline comments to D123720: [VPlan] Replace use of needsVectorIV with VPlan user check..
Apr 28 2022, 9:16 AM · Restricted Project, Restricted Project

Apr 27 2022

Ayal added inline comments to D123005: [VPlan] Use region for each loop in native path..
Apr 27 2022, 3:27 PM · Restricted Project, Restricted Project

Apr 25 2022

Ayal added inline comments to D123537: [VPlan] Model first exit values using VPLiveOut..
Apr 25 2022, 3:16 PM · Restricted Project, Restricted Project

Apr 13 2022

Ayal accepted D123700: [VPlan] Turn external defs in Value -> VPValue mapping..

Ship it!
minor nits.

Apr 13 2022, 1:54 PM · Restricted Project, Restricted Project
Ayal accepted D123720: [VPlan] Replace use of needsVectorIV with VPlan user check..

Looks good to me.
Can this too lead to different - more accurate decisions? If so wonder if that could be tested(?)

Apr 13 2022, 1:35 PM · Restricted Project, Restricted Project
Ayal accepted D123541: [VPlan] Replace remaining use of needsScalarIV..

Looks good to me, curious if it may lead to any regression...
Adding minor nits.

Apr 13 2022, 1:27 PM · Restricted Project, Restricted Project
Ayal accepted D122096: [VPlan] Expand induction step in VPlan pre-header..

This looks good to me, thanks for accommodating, ship it!
Adding minor nits.

Apr 13 2022, 1:08 PM · Restricted Project, Restricted Project
Ayal added a comment to D123541: [VPlan] Replace remaining use of needsScalarIV..

Note that there are some test changes, because we now can correctly look through instructions like truncates to analyze the actual users.

Apr 13 2022, 9:10 AM · Restricted Project, Restricted Project
Ayal added inline comments to D123537: [VPlan] Model first exit values using VPLiveOut..
Apr 13 2022, 1:17 AM · Restricted Project, Restricted Project

Apr 12 2022

Ayal accepted D123457: [VPlan] Initial modeling of middle block in VPlan..

Nice preparation for expanding VPlan's scope to include exit/middle-block after its expansion to include preheader.
Looks good to me, adding minor nits.

Apr 12 2022, 11:39 PM · Restricted Project, Restricted Project
Ayal added inline comments to D122096: [VPlan] Expand induction step in VPlan pre-header..
Apr 12 2022, 10:48 PM · Restricted Project, Restricted Project

Apr 9 2022

Ayal added a comment to D122096: [VPlan] Expand induction step in VPlan pre-header..

Ping :)

Apr 9 2022, 8:19 AM · Restricted Project, Restricted Project

Apr 8 2022

Ayal added inline comments to D121624: [VPlan] Model pre-header explicitly..
Apr 8 2022, 2:11 PM · Restricted Project, Restricted Project

Apr 4 2022

Ayal accepted D121624: [VPlan] Model pre-header explicitly..

This looks fine! Adding a couple of nits and future thoughts:

Apr 4 2022, 6:56 AM · Restricted Project, Restricted Project

Apr 3 2022

Ayal added inline comments to D121624: [VPlan] Model pre-header explicitly..
Apr 3 2022, 2:49 PM · Restricted Project, Restricted Project

Mar 30 2022

Ayal accepted D121623: [LV] Remove unneeded createHeaderBranch.(NFCI).

This is fine, thanks!

Mar 30 2022, 4:41 AM · Restricted Project, Restricted Project

Mar 29 2022

Ayal added inline comments to D121623: [LV] Remove unneeded createHeaderBranch.(NFCI).
Mar 29 2022, 12:35 PM · Restricted Project, Restricted Project
Ayal accepted D121621: [VPlan] Track current vector loop in VPTransformState (NFC)..

This is fine; adding a clarifying comment.

Mar 29 2022, 12:02 PM · Restricted Project, Restricted Project
Ayal accepted D121618: [LV] Move code to place pointer induction increment to VPlan post-processing..

Looks fine to me, with a couple of nits.

Mar 29 2022, 11:15 AM · Restricted Project, Restricted Project

Mar 28 2022

Ayal added inline comments to D122096: [VPlan] Expand induction step in VPlan pre-header..
Mar 28 2022, 3:20 AM · Restricted Project, Restricted Project
Ayal accepted D122095: [VPlan] Place VPExpandSCEVRecipe in pre-header..

Looks good to me!

Mar 28 2022, 1:27 AM · Restricted Project, Restricted Project

Mar 27 2022

Ayal added inline comments to D121624: [VPlan] Model pre-header explicitly..
Mar 27 2022, 11:37 PM · Restricted Project, Restricted Project
Ayal accepted D121617: [LV] Move code to place induction increment to VPlan post-processing..

A good step forward in refactoring and simplifying induction variable recipes, raising thoughts for follow-up steps. Ship it!

Mar 27 2022, 1:25 PM · Restricted Project, Restricted Project
Ayal added inline comments to D121621: [VPlan] Track current vector loop in VPTransformState (NFC)..
Mar 27 2022, 9:16 AM · Restricted Project, Restricted Project
Ayal added inline comments to D121623: [LV] Remove unneeded createHeaderBranch.(NFCI).
Mar 27 2022, 9:16 AM · Restricted Project, Restricted Project
Ayal added inline comments to D121621: [VPlan] Track current vector loop in VPTransformState (NFC)..
Mar 27 2022, 8:13 AM · Restricted Project, Restricted Project
Ayal added inline comments to D121617: [LV] Move code to place induction increment to VPlan post-processing..
Mar 27 2022, 5:24 AM · Restricted Project, Restricted Project

Mar 20 2022

Ayal accepted D121619: [LV] Do not create separate latch block in VPlan::execute..

Nice! Looks good to me.

Mar 20 2022, 4:12 PM · Restricted Project, Restricted Project
Ayal added inline comments to D121618: [LV] Move code to place pointer induction increment to VPlan post-processing..
Mar 20 2022, 4:02 PM · Restricted Project, Restricted Project
Ayal added inline comments to D121617: [LV] Move code to place induction increment to VPlan post-processing..
Mar 20 2022, 2:32 PM · Restricted Project, Restricted Project

Mar 16 2022

Ayal accepted D121615: [VPlan] Add VPWidenPointerInductionRecipe..

Nice clean-up, effectively retiring ILV's widenPHIInstruction(), completing its refactoring into more specific recipes.
Adding various nits.

Mar 16 2022, 12:23 AM · Restricted Project, Restricted Project

Mar 15 2022

Ayal accepted D121613: [LV] Use usesScalars in widenPHIInstruction..

Ship it!

Mar 15 2022, 8:05 AM · Restricted Project, Restricted Project
Ayal accepted D121612: [VPlan] VPInterleaveRecipe only requires the first lane of the address..

Ship it!
Address clearly uses first lane only, as evident in VPInterleaveGroup::execute(), and the other operands Stored values and Mask clearly use all lanes.
This will be exercised/tested by follow-up patch only?

Mar 15 2022, 7:43 AM · Restricted Project, Restricted Project

Mar 10 2022

Ayal accepted D120828: [LV] Create & use VPScalarIVSteps for all scalar users..

Ship it!
Would be good to follow-up with cleaning-up of needsScalarIV/needsVectorIV.

Mar 10 2022, 6:53 AM · Restricted Project, Restricted Project
Ayal accepted D120827: [VPlan] Helper to check if a recipe only uses scalar values of op..

Looks good to me, with some last minor nits.
Can drop 'only' also from title.
Specifying VPScalarIVStepsRecipe::onlyFirstLaneUsed() could be committed separately, it belongs to previous patches introducing VPScalarIVStepsRecipe and/or onlyFirstLaneUsed().

Mar 10 2022, 6:40 AM · Restricted Project, Restricted Project
Ayal added inline comments to D120827: [VPlan] Helper to check if a recipe only uses scalar values of op..
Mar 10 2022, 6:33 AM · Restricted Project, Restricted Project

Mar 9 2022

Ayal added a comment to D120827: [VPlan] Helper to check if a recipe only uses scalar values of op..

Perhaps drop "only" - "usesScalars()" instead of "onlyScalarsUsed()"? As opposed to usesVectors.

Mar 9 2022, 12:14 PM · Restricted Project, Restricted Project
Ayal added inline comments to D120828: [LV] Create & use VPScalarIVSteps for all scalar users..
Mar 9 2022, 11:22 AM · Restricted Project, Restricted Project
Ayal added inline comments to D120827: [VPlan] Helper to check if a recipe only uses scalar values of op..
Mar 9 2022, 10:43 AM · Restricted Project, Restricted Project

Mar 8 2022

Ayal added a comment to D120828: [LV] Create & use VPScalarIVSteps for all scalar users..

This nicely removes from ILV both widenIntOrFpInduction() along with createVectorIntOrFpInductionPHI() which it calls, inlining them into VPlan's VPWidenIntOrFpInductionRecipe::execute(), while postponing buildScalarSteps() for all potential scalar users of the vectorized IV to a VPlan2VPlan transformation/optimization.

Mar 8 2022, 9:32 AM · Restricted Project, Restricted Project
Ayal added a comment to D120827: [VPlan] Helper to check if a recipe only uses scalar values of op..

How about VPScalarIVStepsRecipe, VPBlendRecipe, VPBranchOnMaskRecipe, VPPredInstPHIRecipe, VPExpandSCEVRecipe?

Mar 8 2022, 7:31 AM · Restricted Project, Restricted Project

Mar 6 2022

Ayal added a comment to D118642: [IVDescriptor] Find original 'Previous' for first-order recurrences..

@fhahn This change seems to be hitting an assertion failure in one of our internal tests. I have filed GHI#54223 with a repro. Can you take a look?

Thanks, that should be fixed by de8ac485e5b7

The issue was that the order in the map vector wasn't correct. That has been fixed by remove the entry before re-sinking. This ensures it will be sunk after all earlier instructions have been sunk.

Mar 6 2022, 12:21 AM · Restricted Project, Restricted Project

Mar 1 2022

Ayal accepted D118642: [IVDescriptor] Find original 'Previous' for first-order recurrences..

Looks good to me, with a last nit.

Mar 1 2022, 8:46 AM · Restricted Project, Restricted Project

Feb 28 2022

Ayal added inline comments to D118642: [IVDescriptor] Find original 'Previous' for first-order recurrences..
Feb 28 2022, 8:14 AM · Restricted Project, Restricted Project

Feb 26 2022

Ayal added inline comments to D116288: [VPlan] Add recipe to handle SCEV expansion (NFC)..
Feb 26 2022, 2:06 PM · Restricted Project

Feb 22 2022

Ayal accepted D116288: [VPlan] Add recipe to handle SCEV expansion (NFC)..
Feb 22 2022, 2:09 AM · Restricted Project
Ayal accepted D115953: [VPlan] Introduce recipe to build scalar steps..

Looks good to me, thanks!

Feb 22 2022, 1:45 AM · Restricted Project

Feb 21 2022

Ayal accepted D118051: [VPlan] Remove dead header-phi recipes..

Looks good to me, thanks!

Feb 21 2022, 9:49 AM · Restricted Project

Feb 20 2022

Ayal added a comment to D118051: [VPlan] Remove dead header-phi recipes..

minor nits

Feb 20 2022, 2:28 PM · Restricted Project
Ayal added inline comments to D115953: [VPlan] Introduce recipe to build scalar steps..
Feb 20 2022, 2:13 PM · Restricted Project

Feb 19 2022

Ayal added inline comments to D118051: [VPlan] Remove dead header-phi recipes..
Feb 19 2022, 11:26 PM · Restricted Project

Jan 27 2022

Ayal accepted D118167: [VPlan] Record whether scalar IVs are need in induction recipe. (NFC).

This is fine, couple of final nits; thanks for accommodating!

Jan 27 2022, 9:42 AM · Restricted Project
Ayal added inline comments to D118167: [VPlan] Record whether scalar IVs are need in induction recipe. (NFC).
Jan 27 2022, 8:12 AM · Restricted Project
Ayal accepted D116123: [VPlan] Handle IV vector splat using VPWidenCanonicalIV..

This is fine, with a last nit

Jan 27 2022, 2:31 AM · Restricted Project

Jan 26 2022

Ayal accepted D116554: [VPlan] Use VPlan to check if only the first lane is used..

This is fine, remove EntryVal from buildScalarSteps() and update comment?

Jan 26 2022, 11:34 AM · Restricted Project
Ayal added inline comments to D116123: [VPlan] Handle IV vector splat using VPWidenCanonicalIV..
Jan 26 2022, 11:19 AM · Restricted Project

Jan 25 2022

Ayal added inline comments to D116123: [VPlan] Handle IV vector splat using VPWidenCanonicalIV..
Jan 25 2022, 12:00 AM · Restricted Project

Jan 19 2022

Ayal accepted D117140: [LV] Always create VPWidenCanonicalIVRecipe, optimize away later..

Remove unnecessary early exit.

This is fine, thanks for accommodating, one last thought: does Canonical mean starting at zero, or possibly at EPResumeVal?

I think checking for zero a the moment should suffice, as EPResumeVal will only be set as start value just before epilogue VPlan execution. This should also match the current behavior before the change (and D117551 which now contains the addition if isCanonical). Once a single plan contains both vector loops we might need some changes in that direction though.

Resetting start of canonicalIV to EPResumeVal just before epiloque VPlan execution might invalidate passes that relied on this value being zero, including removeRedundantCanonicalIVs()? A test would be good, either confirming or reassuring :-)

Added an assert in 165e36bf180e.

Jan 19 2022, 10:13 AM · Restricted Project
Ayal added inline comments to D116123: [VPlan] Handle IV vector splat using VPWidenCanonicalIV..
Jan 19 2022, 8:35 AM · Restricted Project
Ayal added inline comments to D117140: [LV] Always create VPWidenCanonicalIVRecipe, optimize away later..
Jan 19 2022, 6:48 AM · Restricted Project
Ayal added a comment to D117140: [LV] Always create VPWidenCanonicalIVRecipe, optimize away later..

Address latest comments, thanks!

Resetting start of canonicalIV to EPResumeVal just before epiloque VPlan execution might invalidate passes that relied on this value being zero, including removeRedundantCanonicalIVs()? A test would be good, either confirming or reassuring :-)

I am not sure if it is possible to add a test that has epilogue vectorization and a VPWidenCanonicalIVRecipe. We only add VPWidenCanonicalIVRecipe when folding the tail. In that case, epilogue vectorization is disabled as there's no epilogue. But we should be able to add a relevant test for the stepvector/build-scalar-steps patches I think.

Jan 19 2022, 1:03 AM · Restricted Project

Jan 18 2022

Ayal accepted D116554: [VPlan] Use VPlan to check if only the first lane is used..

This looks fine, with a minor comment.

Jan 18 2022, 10:27 AM · Restricted Project
Ayal added a comment to D117140: [LV] Always create VPWidenCanonicalIVRecipe, optimize away later..

This is fine, thanks for accommodating, one last thought: does Canonical mean starting at zero, or possibly at EPResumeVal?

I think checking for zero a the moment should suffice, as EPResumeVal will only be set as start value just before epilogue VPlan execution. This should also match the current behavior before the change (and D117551 which now contains the addition if isCanonical). Once a single plan contains both vector loops we might need some changes in that direction though.

Jan 18 2022, 8:48 AM · Restricted Project