On Wed, Jan 17, 2018 at 06:42:33PM +0100, Tom de Vries wrote:
> +static rtx
> +expand_builtin_goacc_parlevel_id_size (tree exp, rtx target, int ignore)
> +{
> +  tree fndecl = get_callee_fndecl (exp);
> +
> +  const char *name;
> +  switch (DECL_FUNCTION_CODE (fndecl))
> +    {
> +    case BUILT_IN_GOACC_PARLEVEL_ID:
> +      name = "__builtin_goacc_parlevel_id";
> +      break;
> +    case BUILT_IN_GOACC_PARLEVEL_SIZE:
> +      name = "__builtin_goacc_parlevel_size";
> +      break;

Can you avoid that many switches on DECL_FUNCTION_CODE?
Like initialize in this one not just name, but also the
fallback_retval and gen_fn variables and just use them later?

> +    default:
> +      gcc_unreachable ();
> +    }
> +
> +  if (oacc_get_fn_attrib (current_function_decl) == NULL_TREE)
> +    {
> +      error ("%s only supported in openacc code", name);

OpenACC ?

> +      return const0_rtx;
> +    }
> +
> +  tree arg = CALL_EXPR_ARG (exp, 0);
> +  if (TREE_CODE (arg) != INTEGER_CST)
> +    {
> +      error ("non-constant argument 0 to %s", name);

%qs instead of %s, 2 times.

> +      return const0_rtx;
> +    }
> +
> +  int dim = TREE_INT_CST_LOW (arg);
> +  switch (dim)
> +    {
> +    case GOMP_DIM_GANG:
> +    case GOMP_DIM_WORKER:
> +    case GOMP_DIM_VECTOR:
> +      break;
> +    default:
> +      error ("illegal argument 0 to %s", name);
> +      return const0_rtx;
> +    }
> +
> +  if (ignore)
> +    return target;
> +
> +  if (!targetm.have_oacc_dim_size ())
> +    {
> +      rtx retval;
> +      switch (DECL_FUNCTION_CODE (fndecl))
> +     {
> +     case BUILT_IN_GOACC_PARLEVEL_ID:
> +       retval = const0_rtx;
> +       break;
> +     case BUILT_IN_GOACC_PARLEVEL_SIZE:
> +       retval = GEN_INT (1);

In addition to moving these assignments to a single switch,
this one can be fallback_retval = const1_rtx;

Otherwise LGTM for stage1.

        Jakub

Reply via email to