Hi!

On 2/1/22 3:48 PM, Segher Boessenkool wrote:
> On Tue, Feb 01, 2022 at 08:49:34AM -0600, Bill Schmidt wrote:
>> I've modified the previous patch to add more explanatory commentary about
>> the number-of-arguments test that was previously confusing, and to convert
>> the switch into an if-then-else chain.  The rest of the patch is unchanged.
>> Bootstrapped and tested on powerpc64le-linux-gnu.  Is this okay for trunk?
>> gcc/
>>      * config/rs6000/rs6000-c.cc (resolve_vec_mul): Accept args and types
>>      parameters instead of arglist and nargs.  Simplify accordingly.  Remove
>>      unnecessary test for argument count mismatch.
>>      (resolve_vec_cmpne): Likewise.
>>      (resolve_vec_adde_sube): Likewise.
>>      (resolve_vec_addec_subec): Likewise.
>>      (altivec_resolve_overloaded_builtin): Move overload special handling
>>      after the gathering of arguments into args[] and types[] and the test
>>      for correct number of arguments.  Don't perform the test for correct
>>      number of arguments for certain special cases.  Call the other special
>>      cases with args and types instead of arglist and nargs.
>> +  if (fcode != RS6000_OVLD_VEC_PROMOTE
>> +      && fcode != RS6000_OVLD_VEC_SPLATS
>> +      && fcode != RS6000_OVLD_VEC_EXTRACT
>> +      && fcode != RS6000_OVLD_VEC_INSERT
>> +      && fcode != RS6000_OVLD_VEC_STEP
>> +      && (!VOID_TYPE_P (TREE_VALUE (fnargs)) || n < nargs))
>>      return NULL;
> Please don't do De Morgan manually, let the compiler deal with it?
> Although even with that the logic is as clear as mud.  This matters if
> someone (maybe even you) will have to debug this later, or modify this.
> Maybe adding some suitably named variables can clarify things  here?

I can de-deMorgan this.  Do you want to see the patch again, or is it okay
with that change?

Thanks!
Bill

>
>> +  if (fcode == RS6000_OVLD_VEC_MUL)
>> +    returned_expr = resolve_vec_mul (&res, args, types, loc);
>> +  else if (fcode == RS6000_OVLD_VEC_CMPNE)
>> +    returned_expr = resolve_vec_cmpne (&res, args, types, loc);
>> +  else if (fcode == RS6000_OVLD_VEC_ADDE || fcode == RS6000_OVLD_VEC_SUBE)
>> +    returned_expr = resolve_vec_adde_sube (&res, fcode, args, types, loc);
>> +  else if (fcode == RS6000_OVLD_VEC_ADDEC || fcode == RS6000_OVLD_VEC_SUBEC)
>> +    returned_expr = resolve_vec_addec_subec (&res, fcode, args, types, loc);
>> +  else if (fcode == RS6000_OVLD_VEC_SPLATS || fcode == 
>> RS6000_OVLD_VEC_PROMOTE)
>> +    returned_expr = resolve_vec_splats (&res, fcode, arglist, nargs);
>> +  else if (fcode == RS6000_OVLD_VEC_EXTRACT)
>> +    returned_expr = resolve_vec_extract (&res, arglist, nargs, loc);
>> +  else if (fcode == RS6000_OVLD_VEC_INSERT)
>> +    returned_expr = resolve_vec_insert (&res, arglist, nargs, loc);
>> +  else if (fcode == RS6000_OVLD_VEC_STEP)
>> +    returned_expr = resolve_vec_step (&res, arglist, nargs);
>> +
>> +  if (res == resolved)
>> +    return returned_expr;
> This is so convoluted because the functions do two things, and have two
> return values (res and returned_expr).
>
>
> Segher

Reply via email to