This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch, fortran] Handle missing optional MASK for intrinsics


On Sun, Dec 30, 2018 at 06:10:14PM +0100, Thomas Koenig wrote:

> Index: gcc/fortran/trans-expr.c
> ===================================================================
> --- gcc/fortran/trans-expr.c	(Revision 267347)
> +++ gcc/fortran/trans-expr.c	(Arbeitskopie)
> @@ -5760,17 +5760,21 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol *
>  	     array-descriptor actual to array-descriptor dummy, see
>  	     PR 41911 for why a check has to be inserted.
>  	     fsym == NULL is checked as intrinsics required the descriptor
> -	     but do not always set fsym.  */
> +	     but do not always set fsym.  
> +	     Also, it is necessary to pass a NULL pointer to library routines
> +	     which usually ignoer optional arguments, so they can handle

s/ignoer/ignore

> +	     these themselves.  */
>  	  if (e->expr_type == EXPR_VARIABLE
>  	      && e->symtree->n.sym->attr.optional
> -	      && ((e->rank != 0 && elemental_proc)
> -		  || e->representation.length || e->ts.type == BT_CHARACTER
> -		  || (e->rank != 0
> -		      && (fsym == NULL
> -			  || (fsym-> as
> -			      && (fsym->as->type == AS_ASSUMED_SHAPE
> -				  || fsym->as->type == AS_ASSUMED_RANK
> -			      	  || fsym->as->type == AS_DEFERRED))))))
> +	      && (((e->rank != 0 && elemental_proc)
> +		   || e->representation.length || e->ts.type == BT_CHARACTER
> +		   || (e->rank != 0
> +		       && (fsym == NULL
> +			   || (fsym-> as

Remove space in 'fsym-> as'

> +			       && (fsym->as->type == AS_ASSUMED_SHAPE
> +				   || fsym->as->type == AS_ASSUMED_RANK
> +				   || fsym->as->type == AS_DEFERRED)))))
> +		  || se->ignore_optional))
>  	    gfc_conv_missing_dummy (&parmse, e, fsym ? fsym->ts : e->ts,
>  				    e->representation.length);
>  	}

(snip)

> +	{
> +	  tree present;
> +	  tree type;
> +
> +	  type = TREE_TYPE (maskse.expr);
> +	  present = gfc_conv_expr_present (maskexpr->symtree->n.sym);
> +	  present = convert (type, present);
> +	  present = fold_build1_loc (input_location, TRUTH_NOT_EXPR, type,
> +				     present);
> +	  ifmask = fold_build2_loc (input_location, TRUTH_ORIF_EXPR,
> +				    type, present, maskse.expr);
> +	}
> +      else
> +	ifmask = maskse.expr;
> +

This block of code appears multiple time in the patch.
I wonder if you should split it out into its own function.

static tree
generate_mask (gfc_expr *mask)   /* Choose whatever name you like.
{

}

You could then just to

ifmask = generate_mask (maskse.expr);


Other than that, the patch looks ok to me.

-- 
steve


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]