On Wed, 20 Dec 2023, Richard Sandiford wrote: > Richard Biener <rguent...@suse.de> writes: > > On Tue, 19 Dec 2023, Andrew Pinski wrote: > > > >> On Tue, Dec 19, 2023 at 2:40?AM Richard Sandiford > >> <richard.sandif...@arm.com> wrote: > >> > > >> > Richard Biener <rguent...@suse.de> writes: > >> > > On Tue, 19 Dec 2023, juzhe.zh...@rivai.ai wrote: > >> > > > >> > >> Hi, Richard. > >> > >> > >> > >> After investigating the codes: > >> > >> /* Return true if EXPR is the integer constant zero or a complex > >> > >> constant > >> > >> of zero, or a location wrapper for such a constant. */ > >> > >> > >> > >> bool > >> > >> integer_zerop (const_tree expr) > >> > >> { > >> > >> STRIP_ANY_LOCATION_WRAPPER (expr); > >> > >> > >> > >> switch (TREE_CODE (expr)) > >> > >> { > >> > >> case INTEGER_CST: > >> > >> return wi::to_wide (expr) == 0; > >> > >> case COMPLEX_CST: > >> > >> return (integer_zerop (TREE_REALPART (expr)) > >> > >> && integer_zerop (TREE_IMAGPART (expr))); > >> > >> case VECTOR_CST: > >> > >> return (VECTOR_CST_NPATTERNS (expr) == 1 > >> > >> && VECTOR_CST_DUPLICATE_P (expr) > >> > >> && integer_zerop (VECTOR_CST_ENCODED_ELT (expr, 0))); > >> > >> default: > >> > >> return false; > >> > >> } > >> > >> } > >> > >> > >> > >> I wonder whether we can simplify the codes as follows :? > >> > >> if (integer_zerop (arg1) || integer_zerop (arg2)) > >> > >> step_ok_p = (code == BIT_AND_EXPR || code == BIT_IOR_EXPR > >> > >> || code == BIT_XOR_EXPR); > >> > > > >> > > Possibly. I'll let Richard S. comment on the whole structure. > >> > > >> > The current code is handling cases that require elementwise arithmetic. > >> > ISTM that what we're really doing here is identifying cases where > >> > whole-vector arithmetic is possible instead. I think that should be > >> > a separate pre-step, rather than integrated into the current code. > >> > > >> > Largely this would consist of writing out match.pd-style folds in > >> > C++ code, so Andrew's fix in comment 7 seems neater to me. > >> > >> I didn't like the change to match.pd (even with a comment on why) > >> because it violates the whole idea behind canonicalization of > >> constants being 2nd operand of commutative and comparison expressions. > >> Maybe there are only a few limited match/simplify patterns which need > >> to add the :c for constants not being the 2nd operand but there needs > >> to be a comment on why :c is needed for this. > > > > Agreed. Note that in theory we of course could define extra > > canonicalization rules for CST op CST in tree_swap_operands_p, > > there are some cases those surivive, mostly around FP and > > dynamic rounding state. I'd rather go that route if we decide > > that match.pd should catch this. > > I don't think that'll help in all cases though. E.g. consider an > unfoldable: > > (or 1 CST) > > where CST is "complicated". We'd probably canonicalise that to: > > (or CST 1) > > And that's good if we have: > > (or (or CST 1) 2) > > since it could be folded to: > > (or CST 3) > > But there are other constants CST2 for which (or CST CST2) is foldable > and (op 1 CST2) isn't. So: > > (or (or 1 CST) CST2) > > would sometimes be foldable in cases where: > > (or (or CST 1) CST2) > > isn't.
OK, I was thinking of only handling VECTOR_CST_NELTS_PER_PATTERN for example as additional (cheap) heuristic so we put VECTOR_CST_DUPLICATE_P second (this would cover the particular cases in this thread). > >> > > >> > But if this must happen in const_binop instead, then we could have > >> > a function like: > >> > >> The reasoning of why it should be in const_binop rather than in match > >> is because both operands are constants. > > > > +1 > > I can see that argument for the traditional case where all constants > can be folded at compile time. But that isn't the case for VLA constants. > (op CST1 CST2) is sometimes not knowable at compile time. And I think > match.pd should apply to those cases just as much as to (op VAR CST). > > VLA vector "constants" are really in intermediate category between > variable and constant. > > The approach that the patch takes is to add a "rule of thumb" that > applies to all (op X CST), regardless of whether X is constant. > It doesn't work by doing constant arithmetic per se. If we add > those rules of thumb here, I think it'll keep growing and growing. But doesn't this mean we can't rely on folding (match.pd) not seeing un-constant-folded operations and thus proper canonicalization? Which means we'd possibly have to alter _all_ (op X CST) cases to use :c? > Thanks, > Richard > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)