[PATCH] D24661: More processors support under AARch64 state for auto detection.

[Renato Golin via llvm-commits]

I agree with Tim. This needs a better place. Decoding all variations
on ARM hardware is bound to explode the number of lines here for a
factor of 10.

Nothing to do with this patch, the code was already ugly before, but
ARM is a special type of monster when it comes to unification of
anything, so it won't make things slightly worse, but a lot worse.

Since this is not fixing a bug or anything, I don't think it's worth
putting as is and making it better later. A simple library to decode
cpuinfo would go a long way of making this manageable, and it should
be an isolated implementation with a simple struct and a few string
refs, nothing fancy.

This would eliminate the need for strcmp, string switches, etc. A
trick like the one we used for target parser would also work (enum
based include files), that's why I suggested moving it there. But I
agree, this has nothing to do with the targets, but the system, so
need to be in a separate place.

cheers,
--renato

On 19 September 2016 at 20:09, Tim Northover <t.p.northover at gmail.com> wrote:
>> Or do you mean the "for" loop body itself is inefficient (and not the for
>> loop being repeated) in the sense that there is lot of strcmp?
>
> It's more the code duplication aspect that bothers me.
>
>> 1. Converting to numbers to be used in lower_bound on a static array needs
>> maintenance (manual sorting). It might be prone to errors especially when
>> the "CPU Part" strings (like 0xd03, etc.) change the sequence in future. One
>> has to be very careful then.
>
> In similar situations in Clang I've used
> "assert(std::is_sorted(...))". Elides to nothing in release builds but
> stops people getting it wrong when modifying the table manually. Using
> TableGen in some manner to generate the search is another option,
> though I'd probably say not worth it here.
>
>> 2. Definitely reduces readability. The current approach is very neat.
>
> I disagree. Looking through dozens of lines that may or may not be
> identical after whatever patches have introduced Samsung, Qualcomm,
> Apple, and whatever parsing is really something I want to avoid.
>
>> 4. The list may not really grow up to many such entries under one single
>> implementer. In my opinion we can live with the penalty given other
>> benefits.
>
> I'm still not seeing the benefits, I'm afraid.
>
>> In fact this whole thing of parsing /proc/cpuinfo itself is inefficient, but
>> cant help in case of ARM. Even querying their Main ID Register (MIDR) or
>> HWCAP is not complete. I just wish we had an system call interface (through
>> libc) to query different aspects of procfs (/proc) - Some sort of a
>> software-cpuid.
>
> Now that I could get behind!
>
> Tim.