On Thu, Jan 12, 2017 at 2:30 PM, Joseph Myers <jos...@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
> jos...@codesourcery.com
