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

[Max Filippov]

Hello,

On Tue, Sep 6, 2016 at 11:42 AM, Oleksij Rempel  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

Yes, these changes are needed, but perhaps not in a form of a patch.
The changed file is a part of the configuration overlay that need to be
applied to binutils, gcc and gdb in order to configure them to generate
code for the specific xtensa core.
We have xtensa support in the crosstool-NG and the Buildroot, both
of which can generate xtensa toolchain using configuration overlay.
Please refer to
 http://wiki.linux-xtensa.org/index.php/Toolchain_and_Embedded_Distributions
for more information about xtensa configuration overlay and its usage
by crosstool-NG and Buildroot.

> Is it possible or welcome to extend gcc to be configurable without
> patching it all the time?

It is definitely welcome and is likely possible.
Please do not forget that both gcc and binutils need to have coherent idea
of the processor they're generating code for.

-- 
Thanks.
-- Max

Advice sought for debugging a lto1 ICE (was: Implement C _FloatN, _FloatNx types [version 6])

[Thomas Schwinge]

Hi!

I trimmed the CC list -- I'm looking for advice about debugging a lto1
ICE.

On Fri, 19 Aug 2016 11:05:59 +, Joseph Myers  
wrote:
> On Fri, 19 Aug 2016, Richard Biener wrote:
> > Can you quickly verify if LTO works with the new types?  I don't see 
> > anything
> > that would prevent it but having new global trees and backends initializing 
> > them
> > might come up with surprises (see tree-streamer.c:preload_common_nodes)
> 
> Well, the execution tests are in gcc.dg/torture, which is run with various 
> options including -flto (and I've checked the testsuite logs to confirm 
> these tests are indeed run with such options).  Is there something else 
> you think should be tested?

As I noted in :

As of the PR32187 commit r239625 "Implement C _FloatN, _FloatNx types", 
nvptx
offloading is broken, ICEs in LTO stream-in.  Probably some kind of 
data-type
mismatch that is not visible with Intel MIC offloading (using the same data
types) but explodes with nvptx.  I'm having a look.

I know how to use "-save-temps -v" to re-run the ICEing lto1 in GDB; a
backtrace of the ICE looks as follows:

#0  fancy_abort (file=file@entry=0x10d61d0 "[...]/source-gcc/gcc/vec.h", 
line=line@entry=727, function=function@entry=0x10d6e3a 
<_ZZN3vecIP9tree_node7va_heap8vl_embedEixEjE12__FUNCTION__> "operator[]") at 
[...]/source-gcc/gcc/diagnostic.c:1414
#1  0x0058c9ef in vec::operator[] 
(this=0x16919c0, ix=ix@entry=185) at [...]/source-gcc/gcc/vec.h:727
#2  0x0058ca33 in vec::operator[] 
(this=this@entry=0x1691998, ix=ix@entry=185) at [...]/source-gcc/gcc/vec.h:1211
#3  0x00c73e54 in streamer_tree_cache_get_tree (cache=0x1691990, 
ix=ix@entry=185) at [...]/source-gcc/gcc/tree-streamer.h:98
#4  0x00c73eb9 in streamer_get_pickled_tree 
(ib=ib@entry=0x7fffceb0, data_in=data_in@entry=0x1691930) at 
[...]/source-gcc/gcc/tree-streamer-in.c:1112
#5  0x008f841b in lto_input_tree_1 (ib=ib@entry=0x7fffceb0, 
data_in=data_in@entry=0x1691930, tag=tag@entry=LTO_tree_pickle_reference, 
hash=hash@entry=0) at [...]/source-gcc/gcc/lto-streamer-in.c:1404
#6  0x008f8844 in lto_input_tree (ib=0x7fffceb0, 
data_in=0x1691930) at [...]/source-gcc/gcc/lto-streamer-in.c:1444
#7  0x00c720d2 in lto_input_ts_list_tree_pointers 
(ib=ib@entry=0x7fffceb0, data_in=data_in@entry=0x1691930, 
expr=expr@entry=0x76993780) at [...]/source-gcc/gcc/tree-streamer-in.c:861
#8  0x00c7444e in streamer_read_tree_body 
(ib=ib@entry=0x7fffceb0, data_in=data_in@entry=0x1691930, 
expr=expr@entry=0x76993780) at [...]/source-gcc/gcc/tree-streamer-in.c:1077
#9  0x008f6428 in lto_read_tree_1 (ib=ib@entry=0x7fffceb0, 
data_in=data_in@entry=0x1691930, expr=expr@entry=0x76993780) at 
[...]/source-gcc/gcc/lto-streamer-in.c:1285
#10 0x008f651b in lto_read_tree (ib=ib@entry=0x7fffceb0, 
data_in=data_in@entry=0x1691930, tag=tag@entry=4, hash=hash@entry=4086308758) 
at [...]/source-gcc/gcc/lto-streamer-in.c:1315
#11 0x008f85db in lto_input_tree_1 (ib=ib@entry=0x7fffceb0, 
data_in=data_in@entry=0x1691930, tag=tag@entry=4, hash=hash@entry=4086308758) 
at [...]/source-gcc/gcc/lto-streamer-in.c:1427
#12 0x008f8673 in lto_input_scc (ib=ib@entry=0x7fffceb0, 
data_in=data_in@entry=0x1691930, len=len@entry=0x7fffceac, 
entry_len=entry_len@entry=0x7fffcea8) at 
[...]/source-gcc/gcc/lto-streamer-in.c:1339
#13 0x005890f7 in lto_read_decls 
(decl_data=decl_data@entry=0x77fc, data=data@entry=0x169d570, 
resolutions=...) at [...]/source-gcc/gcc/lto/lto.c:1693
#14 0x005898c8 in lto_file_finalize 
(file_data=file_data@entry=0x77fc, file=file@entry=0x15eedb0) at 
[...]/source-gcc/gcc/lto/lto.c:2037
#15 0x00589928 in lto_create_files_from_ids 
(file=file@entry=0x15eedb0, file_data=file_data@entry=0x77fc, 
count=count@entry=0x7fffd054) at [...]/source-gcc/gcc/lto/lto.c:2047
#16 0x00589a7a in lto_file_read (file=0x15eedb0, 
resolution_file=resolution_file@entry=0x0, count=count@entry=0x7fffd054) at 
[...]/source-gcc/gcc/lto/lto.c:2088
#17 0x00589e84 in read_cgraph_and_symbols (nfiles=1, 
fnames=0x160e990) at [...]/source-gcc/gcc/lto/lto.c:2798
#18 0x0058a572 in lto_main () at [...]/source-gcc/gcc/lto/lto.c:3299
#19 0x00a48eff in compile_file () at 
[...]/source-gcc/gcc/toplev.c:466
#20 0x00550943 in do_compile () at 
[...]/source-gcc/gcc/toplev.c:2010
#21 toplev::main (this=this@entry=0x7fffd180, argc=argc@entry=20, 
argv=0x15daf20, argv@entry=0x7fffd288) at [...]/source-gcc/gcc/toplev.c:2144
#22 0x00552717 in main (argc=20, argv=0x7fffd288) at 
[...]/source-gcc/gcc/main.c:39

(Comparing to yesterday's r240004, the line number are slightly off, as
I've added some stuff to aid debugging.)

Looking at f

Re: Advice sought for debugging a lto1 ICE (was: Implement C _FloatN, _FloatNx types [version 6])

[Richard Biener]

On Wed, Sep 7, 2016 at 1:52 PM, Thomas Schwinge  wrote:
> Hi!
>
> I trimmed the CC list -- I'm looking for advice about debugging a lto1
> ICE.
>
> On Fri, 19 Aug 2016 11:05:59 +, Joseph Myers  
> wrote:
>> On Fri, 19 Aug 2016, Richard Biener wrote:
>> > Can you quickly verify if LTO works with the new types?  I don't see 
>> > anything
>> > that would prevent it but having new global trees and backends 
>> > initializing them
>> > might come up with surprises (see tree-streamer.c:preload_common_nodes)
>>
>> Well, the execution tests are in gcc.dg/torture, which is run with various
>> options including -flto (and I've checked the testsuite logs to confirm
>> these tests are indeed run with such options).  Is there something else
>> you think should be tested?
>
> As I noted in :
>
> As of the PR32187 commit r239625 "Implement C _FloatN, _FloatNx types", 
> nvptx
> offloading is broken, ICEs in LTO stream-in.  Probably some kind of 
> data-type
> mismatch that is not visible with Intel MIC offloading (using the same 
> data
> types) but explodes with nvptx.  I'm having a look.
>
> I know how to use "-save-temps -v" to re-run the ICEing lto1 in GDB; a
> backtrace of the ICE looks as follows:
>
> #0  fancy_abort (file=file@entry=0x10d61d0 "[...]/source-gcc/gcc/vec.h", 
> line=line@entry=727, function=function@entry=0x10d6e3a 
> <_ZZN3vecIP9tree_node7va_heap8vl_embedEixEjE12__FUNCTION__> "operator[]") at 
> [...]/source-gcc/gcc/diagnostic.c:1414
> #1  0x0058c9ef in vec::operator[] 
> (this=0x16919c0, ix=ix@entry=185) at [...]/source-gcc/gcc/vec.h:727
> #2  0x0058ca33 in vec::operator[] 
> (this=this@entry=0x1691998, ix=ix@entry=185) at 
> [...]/source-gcc/gcc/vec.h:1211

so it wants tree 185 which is (given the low number) likely one streamed by
preload_common_nodes.  This is carefully crafted to _not_ diverge by
frontend (!) it wasn't even designed to cope with global trees being present
or not dependent on target (well, because the target is always the
same! mind you!)

Now -- in theory it should deal with NULLs just fine (resulting in
error_mark_node), but it can diverge when there are additional
compount types (like vectors, complex
or array or record types) whose element types are not in the set of
global trees.
The complex _FloatN types would be such a case given they appear before their
components.  That mixes up the ordering at least.

So I suggest to add a print_tree to where it does the streamer_tree_cache_append
and compare cc1 and lto1 outcome.

The ICE above means the lto1 has fewer preloaded nodes I guess.

Richard.

> #3  0x00c73e54 in streamer_tree_cache_get_tree (cache=0x1691990, 
> ix=ix@entry=185) at [...]/source-gcc/gcc/tree-streamer.h:98
> #4  0x00c73eb9 in streamer_get_pickled_tree 
> (ib=ib@entry=0x7fffceb0, data_in=data_in@entry=0x1691930) at 
> [...]/source-gcc/gcc/tree-streamer-in.c:1112
> #5  0x008f841b in lto_input_tree_1 (ib=ib@entry=0x7fffceb0, 
> data_in=data_in@entry=0x1691930, tag=tag@entry=LTO_tree_pickle_reference, 
> hash=hash@entry=0) at [...]/source-gcc/gcc/lto-streamer-in.c:1404
> #6  0x008f8844 in lto_input_tree (ib=0x7fffceb0, 
> data_in=0x1691930) at [...]/source-gcc/gcc/lto-streamer-in.c:1444
> #7  0x00c720d2 in lto_input_ts_list_tree_pointers 
> (ib=ib@entry=0x7fffceb0, data_in=data_in@entry=0x1691930, 
> expr=expr@entry=0x76993780) at [...]/source-gcc/gcc/tree-streamer-in.c:861
> #8  0x00c7444e in streamer_read_tree_body 
> (ib=ib@entry=0x7fffceb0, data_in=data_in@entry=0x1691930, 
> expr=expr@entry=0x76993780) at 
> [...]/source-gcc/gcc/tree-streamer-in.c:1077
> #9  0x008f6428 in lto_read_tree_1 (ib=ib@entry=0x7fffceb0, 
> data_in=data_in@entry=0x1691930, expr=expr@entry=0x76993780) at 
> [...]/source-gcc/gcc/lto-streamer-in.c:1285
> #10 0x008f651b in lto_read_tree (ib=ib@entry=0x7fffceb0, 
> data_in=data_in@entry=0x1691930, tag=tag@entry=4, hash=hash@entry=4086308758) 
> at [...]/source-gcc/gcc/lto-streamer-in.c:1315
> #11 0x008f85db in lto_input_tree_1 (ib=ib@entry=0x7fffceb0, 
> data_in=data_in@entry=0x1691930, tag=tag@entry=4, hash=hash@entry=4086308758) 
> at [...]/source-gcc/gcc/lto-streamer-in.c:1427
> #12 0x008f8673 in lto_input_scc (ib=ib@entry=0x7fffceb0, 
> data_in=data_in@entry=0x1691930, len=len@entry=0x7fffceac, 
> entry_len=entry_len@entry=0x7fffcea8) at 
> [...]/source-gcc/gcc/lto-streamer-in.c:1339
> #13 0x005890f7 in lto_read_decls 
> (decl_data=decl_data@entry=0x77fc, data=data@entry=0x169d570, 
> resolutions=...) at [...]/source-gcc/gcc/lto/lto.c:1693
> #14 0x005898c8 in lto_file_finalize 
> (file_data=file_data@entry=0x77fc, file=file@entry=0x15eedb0) at 
> [...]/source-gcc/gcc/lto/lto.c:2037
> #15 0x00589928 in lto_create_files_from_ids 
> (fil

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

[augustine.sterl...@gmail.com]

On Tue, Sep 6, 2016 at 11:55 PM, Thomas Schwinge
 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  
> 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.

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.

Surely if someone is building this package, they would have already
built some other software for that particular xtensa target. (Perhaps
as part of a larger debian build?)

Also, this package should probably only be built when targeting this
particular xtensa variant (not xtensa generally). I don't know how one
restricts this in the debian packaging mechanism.

Hope this helps, and I'm happy to answer more questions.

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

[augustine.sterl...@gmail.com]

On Wed, Sep 7, 2016 at 9:21 AM, augustine.sterl...@gmail.com
 wrote:
> Hope this helps, and I'm happy to answer more questions.

Also, one technique commonly used by people who ship software for
Xtensa is to write it such that it could compile for any variant at
all. This requires care, but is quite doable.

Re: Proposal: readable and writable attributes on variables

[Jeff Law]

On 09/01/2016 09:04 AM, Martin Sebor wrote:


Understood.  I think a write-only attribute or type qualifier would
make sense.  Until/unless it's implemented I would recommend to work
around its absence by hiding access to the registers behind a read-
only and write-only functional API.
As you noted earlier Martin, if we bake it into the typesystem, then you 
get the desired warnings when you mix-n-match types.  For that reason I 
see a type qualifier is superior to an attribute.


IIRC the national labs that were looking at the alignment attribute 
essentially came to the same conclusion -- bake it into the core of the 
typesystem and rely on the typesystem to ensure you don't lose the data.


Jeff