Re: why do we need xtensa-config.h?

["augustine dot sterling at gmail dot com" <augustine dot sterling at gmail dot com>]

On Thu, Sep 8, 2016 at 8:14 AM, Oleksij Rempel <linux@rempel-privat.de> wrote:
> Am 07.09.2016 um 18:21 schrieb augustine.sterling@gmail.com:
>> On Tue, Sep 6, 2016 at 11:55 PM, Thomas Schwinge
>> <thomas@codesourcery.com> wrote:
>>> Hi!
>>>
>>> Neither do I really know anything about Xtensa, nor do I have a lot of
>>> experience in these parts of GCC back ends, but:
>>
>> There is a lot of background to know here. Unfortunately, I have no
>> familiarity with making debian packages, so I'm unfamiliar with that
>> side of it.
>>
>> First--and perhaps most important--the current method of configuring
>> GCC for xtensa targets has worked well for nearly two decades. As far
>> as I know, it is rare to encounter problems. Because of that, the bar
>> to change it will probably be fairly high to change it.
>>
>>> On Tue, 6 Sep 2016 20:42:53 +0200, Oleksij Rempel <linux@rempel-privat.de> wrote:
>>>> i'm one of ath9k-htc-firmware developers. Currently i'm looking for the
>>>> way to provide this firmware as opensource/free package for debian. Main
>>>> problem seems to be the need to patch gcc xtensa-config.h to make it
>>>> suitable for our CPU.
>>>>
>>>> I have fallowing questions:
>>>>
>>>> do we really need this patch?
>>>> https://github.com/qca/open-ath9k-htc-firmware/blob/master/local/patches/gcc.patch
>>>
>>> That I can't tell.  ;-)
>>
>> You need something like that patch, for sure.
>>
>>>> Is it possible or welcome to extend gcc to be configurable without
>>>> patching it all the time?
>>>
>>> Yes, I would think.  The macros modified in the above patch to GCC's
>>> include/xtensa-config.h file look like these ought to be modifiable with
>>> -m* options defined by the Xtensa back end, and you'd then assign
>>> specific defaults to a specific CPU variant, and build GCC (or build a
>>> multilib) for that configuration.
>>
>> Today, there are literally hundreds of variants of the xtensa cpu
>> actually realized and in use. Having a list of all those variants and
>> their defaults inside gcc would be awkward and unwieldy.
>>
>> But--and here's the rub--literally tomorrow, someone could design a
>> hundred more that are different from all of the ones already out
>> there. There is literally an unlimited number of potential variants,
>> each with potentially brand new, never conceived instructions. (Adding
>> clever custom instructions is xtensa's raison d'etre.)
>>
>> With the current configurability mechanism, supporting all of those
>> variants inside gcc (and, in fact, the rest of the gnu-toolchain) is
>> simply a matter of using the correct xtensa-config.h for that
>> particular variant. If we were to go with the "-m with defaults"
>> mechanism, we would need some way of adding the defaults for the new
>> variant to gcc.
>>
>> But that is patching gcc also, and once you go there, you may as well
>> use the original method.
>>
>>>
>>> This file include/xtensa-config.h is #included in
>>> gcc/config/xtensa/xtensa.h and libgcc/config/xtensa/crti.S,
>>
>> Note that "-m" options can't change the instructions in crti.S and
>> lib?funcs.S, but macros can and do.
>>
>>
>>
>> On the debian packaging side. Forgive me for my ignorance on the
>> topic; I don't know that the tool-flow is, or what the requirements
>> are. As far as I am aware, this is the first time someone has tried to
>> make a debian package for xtensa.
>
> The point is to provide a package for "free" repository. It means, any
> one should be able to use "apt-get source package_name", patch it and
> recompile it from source.
>
> Right now it would work, but the package scripts should download
> toolchain source, patch and compile it and the compile actual firmware.
> This is wary high overhead.
>
> This is why i asked my self, why the toolchain can't be modular or
> configurable enough to work without patching and recompiling it.
>
>> Anyway, I wouldn't expect patching gcc (or configuring it) for an
>> individual package is the right thing. It should probably happen as
>> part of some kind of "setup toolchain" step.
>>
>> Typically, patching gcc for a xtensa config happens just once
>> immediately after designing the processor, or--if you aren't the
>> designer yourself--when one starts development for that variant.
>
> after quick look i noticed that:
> XSHAL_USE_ABSOLUTE_LITERALS affects TRAMPOLINE_SIZE. This seems to be
> hardcoded every where in gcc, so can't be changed dynamically?

TRAMPOLINE_SIZE is used in quite a few places (so beyond my authority
to accept patches), but I suspect it would be acceptable to put a
function behind TRAMPOLINE_SIZE that calculated the value dynamically.

>
> XCHAL_HAVE_MUL32_HIGH probably can be changed dynamically.
> XCHAL_ICACHE_SIZE and XCHAL_DCACHE_SIZE will enable or disable part of
> __xtensa_sync_caches, why not to make it dynamically as command line option?
>
>
> IMO most of xtensa-config.h can be changed on runtime. Or do i miss some
> thing?

Unless we can make it all of what xtensa-config.h provides, we don't
really solve the problem.

Also, I'm wary of adding command line options that are required to get
correct object-code. GCC can't validate the options against the
hardware it is actually targeting (that's what xtensa-config.h
actually tells it, so without a correct one, it can't know).

This also puts some burden on every one who uses it--passing fifteen
-mfoo=bar options is likely quite error prone and prevents someone
from just typing "gcc hello.c && ./a.out". This will also only work if
the source code one is compiling doesn't use these macros either, and
there is no way to check at compile time.

So, for example, it wouldn't work for gcc bootstrap because
lib1funcs.S and the c-runtime rely on the macros defined in
xtensa-config.h.

So I'm wary of this approach.

Nonetheless, I would accept a patch that adds these -m options as long
as they default to the values obtained from xtensa-config.h, and
provides a reasonable method for the user to determine and pass all of
the appropriate configuration values.