Re: [PATCH 15/16] RTL frontend (rtl1), on top of dump reader

[Bernd Schmidt <bschmidt@redhat.com>]

Let me just make a first pass over this for minor/obvious issues.

> +we have little control of the input to that specific pass.  We

"control over" maybe?

> +The testsuite is below @file{gcc/testsuite/rtl.dg}.

Not sure this needs to be in the manual (I have similar doubts about the 
entire motivation section, but I guess we can keep it). Also, "below"?

> +/* rtl-error.c - Replacement for errors.c for use by RTL frontend
> +   Copyright (C) 2016 Free Software Foundation, Inc.

Why have this and not use the normal machinery?

> +
> +static bool
> +rtl_langhook_handle_option (
> +    size_t scode,
> +    const char *arg,
> +    int value ATTRIBUTE_UNUSED,
> +    int kind ATTRIBUTE_UNUSED,
> +    location_t loc ATTRIBUTE_UNUSED,
> +    const struct cl_option_handlers *handlers ATTRIBUTE_UNUSED)

Please line up the arguments, including the first, with the open paren.
For hooks I think we're converging towards just not naming unused args.

> +
> +  /*  If -fsingle-pass=PASS_NAME was provided, locate and run PASS_NAME
> +      on cfun, as created above.  */

Comment text indented too much.

> +/* Implementation of LANG_HOOKS_TYPE_FOR_MODE, taken from Go frontend.  */
> +
> +static tree
> +rtl_langhook_type_for_mode (machine_mode mode, int unsignedp)
> +{
> +  tree type;
> +  /* Go has no vector types.  Build them here.  FIXME: It does not
> +     make sense for the middle-end to ask the frontend for a type
> +     which the frontend does not support.  However, at least for now
> +     it is required.  See PR 46805.  */

Hmm, not sure it makes much sense to have comments referencing Go.

> +  if (VECTOR_MODE_P (mode))
> +    {
> +      tree inner;
> +
> +      inner = rtl_langhook_type_for_mode (GET_MODE_INNER (mode), unsignedp);
> +      if (inner != NULL_TREE)
> +	return build_vector_type_for_mode (inner, mode);
> +      return NULL_TREE;
> +    }
> +
> +  // FIXME: This static_cast should be in machmode.h.
> +  enum mode_class mc = static_cast<enum mode_class>(GET_MODE_CLASS(mode));
> +  if (mc == MODE_INT)
> +    return rtl_langhook_type_for_size(GET_MODE_BITSIZE(mode), unsignedp);

Also not really following our coding standards. Presumably this is all 
copied?

> +#include <mpfr.h>

The include list probably wants checking whether everything is really 
necessary; this one stood out as a bit odd (several files).
> +
> +#ifndef GCC_RUN_ONE_RTL_PASS_H
> +#define GCC_RUN_ONE_RTL_PASS_H
> +
> +extern void run_one_rtl_pass_by_name (const char *pass_name);
> +
> +#endif /* GCC_RUN_ONE_RTL_PASS_H */

Do we really need an entire header file for a single declaration?


Bernd