> On Mar 5, 2015, at 9:14 AM, Terry Guo <flame...@gmail.com> wrote:
>
>>
>> Thanks Terry (and everyone else) for explaining why we want to do this in
>> the driver. The substance of the patch looks good to me, and below are some
>> comments and nit-picks. (Also, I'm not an ARM maintainer, so this is a
>> review, not an approval to commit).
>>
>> Please make sure to update changelog before committing.
>>
>
> Thanks Maxim, your comments are great. I accepted all of them and
> commented two of them that I am not clear.
>
> <<snapped>>
>>> +
>>> +struct arm_arch_core_flag
>>> +{
>>> + const char *const name;
>>> + const unsigned long flags;
>>> +};
>>> +
>>> +static const struct arm_arch_core_flag arm_arch_core_flags[] =
>>> +{
>>> +#undef ARM_CORE
>>> +#define ARM_CORE(NAME, X, IDENT, ARCH, FLAGS, COSTS) \
>>> + {NAME, FLAGS | FL_FOR_ARCH##ARCH},
>>> +#include "arm-cores.def"
>>> +#undef ARM_CORE
>>> +#undef ARM_ARCH
>>> +#define ARM_ARCH(NAME, CORE, ARCH, FLAGS) \
>>> + {NAME, FLAGS},
>>> +#include "arm-arches.def"
>>> +#undef ARM_ARCH
>>> + {NULL, 0}
>>> +};
>>
>> Did you consider implications from mixing ARCHes and CPUs in the same array?
>> It should not be a problem, but would you please double-check that cases
>> like "-march=cortex-a15" are properly caught as errors elsewhere in the
>> driver?
>>
>
> Not sure I follow you correctly here. This array is just used for my
> new arm_target_thumb_only function. It isn't used by any another code
> in gcc. So I don't think mixing them will break gcc option check
> mechanism. I tried below command and I can get error message:
>
> $ ./install-native/bin/arm-none-eabi-gcc -march=cortex-a15 x.c -S -mthumb
> arm-none-eabi-gcc: error: unrecognized argument in option '-march=cortex-a15'
I'm just being paranoid here. The above check you did is all I wanted.
>
>>> #endif
>>> diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
>>> index 28ffe52..325a81c 100644
>>> --- a/gcc/config/arm/arm-protos.h
>>> +++ b/gcc/config/arm/arm-protos.h
>>> @@ -325,75 +325,6 @@ extern const char *arm_rewrite_selected_cpu (const
>>> char *name);
>>>
>>> extern bool arm_is_constant_pool_ref (rtx);
>>>
>>> -/* Flags used to identify the presence of processor capabilities. */
>>
>> You've lost this comment in the new file. Was it intentional?
>>
>
> The line is used as first line in new file arm-flags.h.
Ack.
Thanks,
--
Maxim Kuvyrkov
www.linaro.org
