- User Since
- Jul 12 2015, 1:48 PM (402 w, 5 d)
Sun, Mar 26
Thu, Mar 23
Tue, Mar 21
Sun, Mar 19
This is fine, adding a minor nit.
Couple of nits.
Fri, Mar 17
Tue, Mar 14
Mon, Mar 13
Wed, Mar 8
Nice cleanup! Indicate patch is [NFC]?
Part of moving predicated replicate recipes to either contain a mask operand or be embedded within a replicating region, changing their internal IsPredicated to represent the former but not the latter.
Looks good to me, thanks!
Adding a couple of final nits.
Tue, Mar 7
This change makes a lot of sense, except for getBackedgeValue() - see comment.
Mon, Mar 6
It's hard for getOpenCLKernelParameterType() to detect and diagnose invalid pointer cases w/o context (of an enclosing struct or not), but it's easy to detect valid pointer cases for v2.0+ and return ValidKernelParam.
Sun, Mar 5
Feb 20 2023
Feb 19 2023
Indeed better compute such information on demand than cache it! Looks good to me, adding a couple of nits.
Feb 15 2023
Feb 14 2023
Use -verify=expected,ocl12 instead of #ifdef'ing the test to check diagnostics emitted for 1.2 but not emitted for 2.0.
Feb 13 2023
Updated version merges the if's and checks tests for both 1.2 and 2.0.
Feb 12 2023
Feb 9 2023
Feb 7 2023
This looks good to me, thanks! Adding couple of final nits.
Feb 5 2023
Feb 1 2023
Adding a couple of nits. The optional functionality added cannot yet be exercised (with a test)?
Jan 31 2023
A very nice step forward! Raises a few thoughts regarding recipes of predicated instructions, arguably independent of the patch.
Jan 24 2023
Jan 19 2023
Looks good to me.
May indicate in the commit message that this addresses the iterative nature of sink scalar operands where it is invoked repeatedly.
Looks good to me, with a few minor nits.
Thanks for adding, looks good to me, ship it!
Jan 18 2023
Jan 17 2023
Looks good to me, thanks for adding the test!
Jan 16 2023
OK, thanks for clarifying.
It's somewhat confusing to have the order of parameters dictate which VPValue is being constructed, but the alternative of having each subclass provide its VPDefID seems less appealing.
Perhaps once a common base class is introduced to take care of all recipes that also inherit from VPValue it could pass VPVRecipeSC explicitly when constructing VPValue, rather than relying on this order.
Jan 15 2023
Jan 1 2023
Dec 31 2022
Dec 27 2022
Dec 26 2022
Dec 20 2022
This looks even better with the added tests, and the last issue @venkataramanan.kumar.llvm raised regarding replicated end values seems acceptable.
SCEV expanding the end values is worth a TODO? Added a couple of thoughts that can be addressed separately.
Looks good to me, thanks.
Looks good to me, adding various nits.
Dec 17 2022
Dec 11 2022
Nice catch, ship it!
Dec 10 2022
Ship it, thanks!
Add [NFC] to title?
Relate to recent IV recipe patches in commit message?
Dec 5 2022
Dec 4 2022
Looks good to me, thnks!
Adding a minor optional nit.
Nov 27 2022
This looks good to me, ship it!
Nice simplification, thanks for following up!
Thanks for following up! Adding some thoughts to consider.
Nov 15 2022
This looks good to me, thanks. Adding various nits, mostly irrespective of this patch.
Oct 23 2022
A more accurate name for getDef() may then be getDefiningRecipe(), as in MLIR's Value::getDefiningOp().
Comment needs to be updated.
Ahh, this brings up some further thoughts...
Oct 11 2022
Thanks for accommodating, this looks good to me, with a couple of final nits.
@rengolin, @bmahjour, @venkataramanan.kumar.llvm - any further comments or ok with you to accept?
Oct 5 2022
Good improvement, adding various nits.
Oct 2 2022
Sep 22 2022
This makes good forward progress, adding a couple of nits.
Sep 20 2022
This is essentially saying that for VF=1 every recipe is uniform (across VF lanes), or none are.
Perhaps this should be done more 'uniformly', as in CM.isUniformAfterVectorization(I, VF) which returns true if (VF.isScalar()) regardless of I.
Underlying value is designed to only be used for opcode, type info, metadata and so on, for any recipe; the operands and uses are modeled in VPlan, for any recipe; worth clarifying?
LGTM, adding minor nits.
LGTM! Adding a minor nit.
Sep 19 2022
Nice refactoring - potentially closing a gap between VPlan and original post-vectorization IR sink scalar operands?
Sep 13 2022
Sep 11 2022
Sep 7 2022
Sep 6 2022
Aug 31 2022
Thanks for addressing, looks good to me, adding minor last nits.