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

[Arnd Bergmann]

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?

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.

> 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