Re: [PATCH,rs6000] Add documentation to describe implicit handling of command-line target options

[Segher Boessenkool <segher at kernel dot crashing dot org>]

Hi Kelvin,

On Fri, Mar 17, 2017 at 12:27:41PM -0600, Kelvin Nilsen wrote:
> This patch adds comments to clarify the automatic setting and clearing
> of target attribute flags in order to assure consistency between
> configuration settings and between multiple interrelated compilation
> target options.  Particular attention is given to the target options
> that affect the C preprocessor macros that are automatically defined to
> denote support is enabled for particular target options.

Thanks for doing this.

> +     3. If either of the above two conditions apply except that the
> +	TARGET_DEFAULT macro is defined to equal zero, and
> +	TARGET_POWERPC64 and
> +	a) BYTES_BIG_ENDIAN and the flag to be enabled is either
> +	   MASK_PPC_GVXOPT or MASK_POWERPC64 (flags for "powerpc64"
> +	   target), or

GFXOPT?

> +     4. If a cpu has been requested with a -mcpu=target command-line option
> +	and this cpu has not been disqualified due to shortcomings of the
> +	binary tools,

[Not directly related to this patch of course.]  Is there any good reason
why we do not require a new enough binutils?  During development there are
issues, sure, but why should we for example not require a binutils that
supports POWER8, these days?

> +     2. If TARGET_P8_VECTOR is off.
> +  */

Comment closing does not go on a separate line.

>    /* options from the builtin masks.  */
> +  /* Note that RS6000_BTM_SPE is enabled only if TARGET_SPE
> +     (e.g. -mspe).  */
>    if ((bu_mask & RS6000_BTM_SPE) != 0)
>      rs6000_define_or_undefine_macro (define_p, "__SPE__");
> +  /* Note that RS6000_BTM_PARIED is enabled only if
> +     TARGET_PAIRED_FLOAT is enabled (e.g. -mpaired).  */
>    if ((bu_mask & RS6000_BTM_PAIRED) != 0)
>      rs6000_define_or_undefine_macro (define_p, "__PAIRED__");
> +  /* Note that RS6000_BTM_CELL is enabled only if (rs6000_cpu ==
> +     PROCESSOR_CELL) (e.g. -mcpu=cell).  */
>    if ((bu_mask & RS6000_BTM_CELL) != 0)
>      rs6000_define_or_undefine_macro (define_p, "__PPU__");

I'm not sure these few are useful, they are pretty obvious?  But okay
(if you fix the typo, "PARIED").

Okay with those typoes fixed.

These comments describe what the existing code does.  Did you find anything
that seems wrong or questionable?


Segher