on 2022/9/30 01:11, Segher Boessenkool wrote:
> On Thu, Sep 29, 2022 at 01:45:16PM +0800, Kewen.Lin wrote:
>> I found this flag is mainly related to tune setting and spotted that we have
>> some code
>> for tune setting when no explicit cpu is given.
>>
>> ...
>>
>> else
>> {
>> size_t i;
>> enum processor_type tune_proc
>> = (TARGET_POWERPC64 ? PROCESSOR_DEFAULT64 : PROCESSOR_DEFAULT);
>>
>> tune_index = -1;
>> for (i = 0; i < ARRAY_SIZE (processor_target_table); i++)
>> if (processor_target_table[i].processor == tune_proc)
>> {
>> tune_index = i;
>> break;
>> }
>> }
>
> Ah cool, that needs fixing yes.
>
>> --- a/gcc/config/rs6000/rs6000.cc
>> +++ b/gcc/config/rs6000/rs6000.cc
>> @@ -3702,7 +3702,7 @@ rs6000_option_override_internal (bool global_init_p)
>> else
>> {
>> /* PowerPC 64-bit LE requires at least ISA 2.07. */
>> - const char *default_cpu = (!TARGET_POWERPC64
>> + const char *default_cpu = (!TARGET_POWERPC64 && TARGET_32BIT
>> ? "powerpc"
>> : (BYTES_BIG_ENDIAN
>> ? "powerpc64"
>
> ... but not like that. If this snippet should happen later just move it
> later. Or introduce a new variable to make the control flow less
> confused. Or something else. But don't make the code more complex,
> introducing more special cases like this.
Agree, the diff was mainly to check if it's the root cause. I think we
need to place TARGET_POWERPC64 enablement for 64 bit before this hunk,
I've adjusted it in the new version, will post it once it's full tested.
>
>> +#ifdef OS_MISSING_POWERPC64
>> + else if (OS_MISSING_POWERPC64)
>> + /* It's unexpected to have OPTION_MASK_POWERPC64 on for OSes which
>> + miss powerpc64 support, so disable it. */
>> + rs6000_isa_flags &= ~OPTION_MASK_POWERPC64;
>> +#endif
>
> All silent stuff is always bad.
>
OK, with more testings for replacing warning instead of silently disablement
I noticed that some disablement is needed, one typical case is -m32 compilation
on ppc64, we have OPTION_MASK_POWERPC64 on from TARGET_DEFAULT which is used
for initialization (It makes sense to have it on in TARGET_DEFAULT because
of it's 64 bit cpu). And -m32 compilation matches OS_MISSING_POWERPC64
(!TARGET_64BIT), so it's the case that we have an implicit OPTION_MASK_POWERPC64
on and OS_MISSING_POWERPC64 holds, but it's unexpected not to disable it but
warn it.
BR,
Kewen
> If things are done well, we will end up with *less* code than what we
> had before, not more!
>
>
> Segher
