[PATCH 2/6] RISC-V Port: gcc

[Andrew Waterman]

On Thu, Jan 12, 2017 at 2:30 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Wed, 11 Jan 2017, Palmer Dabbelt wrote:
>
>> +static void
>> +riscv_parse_arch_string (const char *isa, int *flags)
>
> This should be passed the location from riscv_handle_option...
>
>> +      error ("-march=%s: ISA string must begin with rv32 or rv64", isa);
>
>  ... so you can use error_at with an explicit location (for all errors in
> thsi function).
>
>> +static bool
>> +riscv_handle_option (struct gcc_options *opts,
>> +                  struct gcc_options *opts_set ATTRIBUTE_UNUSED,
>> +                  const struct cl_decoded_option *decoded,
>> +                  location_t loc ATTRIBUTE_UNUSED)
>
> The location will no longer then be ATTRIBUTE_UNUSED.

Will do.

>
>> +             supported_defaults="abi arch tune"
>
> So you have three supported defaults here ...
>
>> +/* Support for a compile-time default CPU, et cetera.  The rules are:
>> +   --with-arch is ignored if -march is specified.
>> +   --with-abi is ignored if -mabi is specified.
>> +   --with-tune is ignored if -mtune is specified.  */
>> +#define OPTION_DEFAULT_SPECS \
>> +  {"march", "%{!march=*:-march=%(VALUE)}" }, \
>> +  {"mabi", "%{!mabi=*:-mabi=%(VALUE)}" }, \
>> +  {"tune", "%{!mtune=*:-mtune=%(VALUE)}" }, \
>> +  {"arch", "%{!march=*:-march=%(VALUE)}" }, \
>> +  {"abi", "%{!mabi=*:-mabi=%(VALUE)}" }, \
>
>  ... why do you need the "march" and "mabi" entries here, when I'd expect
> three entries as well?

Indeed, we do not.

>
>> +#undef ASM_SPEC
>> +#define ASM_SPEC "\
>> +%(subtarget_asm_debugging_spec) \
>> +%{fPIC|fpic|fPIE|fpie:-fpic} \
>
> I'd expect you to use FPIE_OR_FPIC_SPEC here to enable
> --enable-default-pie.

Thanks for pointing this out.  We missed that this macro was
introduced after we started the port.

>
> --
> Joseph S. Myers
> joseph@codesourcery.com