On 06/13/2013 09:11 AM, Aldy Hernandez wrote:
> The whole slew of these cases have a lot of duplicated code. For instance,
> BUILT_IN_CILKPLUS_SEC_REDUCE_MIN is the same as
> BUILT_IN_CILKPLUS_SEC_REDUCE_MAX, the only difference being GT_EXPR vs
> LT_EXPR. Surely you could do something like:
>
> if (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MIN
> || an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MAX) {
> enum tree_code code;
> if (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MIN)
> code = GT_EXPR;
> else
> code = LT_EXPR;
> // stuff
> }
>
> The compiler should be able to optimize the above, but even if it couldn't, I
> am willing to compare twice and save lines and lines of code.
>
> Similarly for SEC_REDUCE_ANY_ZERO/SEC_REDUCE_ANY_NONZERO,
> SEC_REDUCE_ALL_ZERO/SEC_REDUCE_ALL_NONZERO, SEC_REDUCE_ADD/SEC_REDUCE_MUL, etc
> etc etc.
Yep. It's at this point that I normally start using the idiom
switch (an_type)
{
case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN:
code = GT_EXPR;
goto do_min_max;
case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX:
code = LT_EXPR;
goto do_min_max;
do_min_max:
// stuff
So much the better if you can include the other SEC_* cases in that switch too,
doing one compiler-controlled dispatch.
It also occurs to me to wonder why you're building your own COND_EXPR here,
with the comparison, rather than using MIN/MAX_EXPR...
r~
