Re: [PATCH v2 25/35] nds32: Build infrastructure

[Greentime Hu]

2017-11-30 17:30 GMT+08:00 Arnd Bergmann <arnd@xxxxxxxx>:
> On Thu, Nov 30, 2017 at 6:48 AM, Greentime Hu <green.hu@xxxxxxxxx> wrote:
>> 2017-11-30 4:27 GMT+08:00 Arnd Bergmann <arnd@xxxxxxxx>:
>>> On Wed, Nov 29, 2017 at 3:10 PM, Greentime Hu <green.hu@xxxxxxxxx> wrote:
>>>> 2017-11-29 19:57 GMT+08:00 Arnd Bergmann <arnd@xxxxxxxx>:
>
>>> When you put them in a sorted list like I mentioned for simplicity, you
>>> could reduce the confusion by naming them differently, e.g.
>>> CONFIG_CPU_N10_OR_NEWER.
>>>
>>> Having only the CPU_CACHE_NONALIASING option is fine if you
>>> never need to make any other decisions based on the CPU core
>>> type, but then the help text should describe specifically which cases
>>> are affected (N10/N13/D13 with 4K page size), and you can decide to
>>> hide the option and make it always-on when using 8K page size.
>>>
>>>        Arnd
>>
>>
>> Hi, Arnd:
>>
>> I think I can use this name "CPU_V3" for all nds32 v3 compatible cpu.
>> It will be implemented like this.
>
> I think I'm still a bit confused about the relation between CPU cores
> and architecture levels. Is it correct to say that there are orthogonal,
> and that you can have e.g. an N10 core implementing either nds32v2
> or nds32v3?
>

Yup, we did having N10 cores are implementing either nds32v3 or
nds32v2, but nds32v2 are not used anymore.
We can assume every nds32 cores are v3.

> There is nothing wrong with that of course, it's just not what I
> expected from having worked with other architectures.
>
> I also see that GCC has no pipeline specific optimizations for
> specific cores, it just understands the differences between the
> architecture levels, so at least today there is way to pass e.g.
> "-march=nds32v2 -mtune=d15" to generate code that would
> work on both v2 and v3 but be optimized for d15.

Thanks. We will work on that.

>> config HWZOL
>>         bool "hardware zero overhead loop support"
>>         depends on CPU_D10 || CPU_D15
>>         default n
>>         help
>>           A set of Zero-Overhead Loop mechanism is provided to reduce the
>>           instruction fetch and execution overhead of loop-control instructions.
>>           It will save 3 registers($LB, $LC, $LE) for context saving if say Y.
>>           You don't need to save these registers if you can make sure your user
>>           program doesn't use these registers.
>>
>>           If unsure, say N.
>>
>> config CPU_CACHE_NONALIASING
>>         bool "Non-aliasing cache"
>>         depends on !CPU_N10 && !CPU_D10
>>         default n
>>         help
>>           If this CPU is using VIPT data cache and its cache way size is larger
>>           than page size, say N. If it is using PIPT data cache, say Y.
>>
>>           If unsure, say N.
>
> This looks ok, yes, but as Geert said, it would seem more intuitive to
> write it as
>
> config CPU_CACHE_ALIASING
>          bool "Aliasing VIPT cache"
>          depends on CPU_N10 || CPU_D10
>
>> choice
>>         prompt "CPU type"
>>         default CPU_V3
>> config CPU_N15
>>         bool "AndesCore N15"
>>         select CPU_CACHE_NONALIASING
>> config CPU_N13
>>         bool "AndesCore N13"
>>         select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>> config CPU_N10
>>         bool "AndesCore N10"
>> config CPU_D15
>>         bool "AndesCore D15"
>>         select CPU_CACHE_NONALIASING
>> config CPU_D10
>>         bool "AndesCore D10"
>> config CPU_V3
>>         bool "AndesCore v3 compatible"
>>         select ANDES_PAGE_SIZE_4KB
>> endchoice
>
> Two points here:
>
> - Generally you should not mix 'select' and 'depends on' like this.
>   Either you make the cache aliasing a user visible option that
>   uses 'depends on' with a combination of CPU cores, or you
>   make it a hidden option (with no string after the "bool" keyword)
>   that always gets selected from the per-cpu options.
>
> - There is a  little-known trick with choice statements that allows
>   you to use 'tristate' instead of 'bool' in the choice. In that case,
>   you can enable multiple options together as long as all of them
>   are 'm'.
>
>          Arnd


Thanks.
CPU_CACHE_ALIASING is more intuitive. I will apply it.