Re: backporting fixes for xtensa to stable branches
On Tue, May 30, 2017 at 3:26 PM, Max Filippov wrote: > Hi Sterling, > > for xtensa we have a number of bugfixes in the mainline that were never > backported to the stable branches. It'd be great having them in the stable > gcc releases instead of carrying the fixes in various toolchain builders. > Would it be ok to do the following backports? Yes. All backports are absolutely fine.
Re: why do we need xtensa-config.h?
On Wed, Sep 21, 2016 at 1:59 AM, Oleksij Rempel wrote: > So, first step is done. Our firmware is using GCC 6.2. > > If i see it correctly, main problem are binutils. First of all we need > custom binutils to compile GCC assambler code with custom opcodes - it's > a horror. > What would be better way to solve it? Have kind of in-project plugins > for binutils and gcc? If this is actually possible to solve it.. :( I don't know of an easy way. A plug-in mechanism for binutils would be a very large amount of work, and would require some pretty broad approvals.
Re: why do we need xtensa-config.h?
On Thu, Sep 8, 2016 at 8:14 AM, Oleksij Rempel wrote: > Am 07.09.2016 um 18:21 schrieb 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. > > 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
Re: why do we need xtensa-config.h?
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: why do we need xtensa-config.h?
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: xtensa PR65730
On Sat, Apr 11, 2015 at 12:43 AM, Max Filippov wrote: > On Sat, Apr 11, 2015 at 2:16 AM, augustine.sterl...@gmail.com > wrote: >> On Fri, Apr 10, 2015 at 1:18 PM, Max Filippov wrote: >>> How can we have a mulsi3 pattern that don't get expanded until it's >>> optimized, and only gets expanded to a call if it couldn't be optimized? >> >> I'm not completely sure, but I think you want to use OPTAB_LIB as you >> described above, and eliminate the TARGET_MUL32 condition in the >> pattern. > > In that case mull is emitted for normal uses of multiplication. You will have to play around with it then. One possibility is to catch the simple cases when the instruction is originally expanded. That is, generate shifts there. You won't get all of the performance you would from the optimizer, but simple cases like even powers of two one shouldn't be too bad.
Re: xtensa PR65730
On Fri, Apr 10, 2015 at 1:18 PM, Max Filippov wrote:
> Ok, then I see why this doesn't happen: mulsi3 pattern matching is
> conditional on TARGET_MUL32, so when TARGET_MUL32 ==0 and
> expand_simple_binop emits a call to a helper it's not considered
> mulsi3, it's just a call:
>
> (call_insn/u 17 15 18 3 (set (reg:SI 10 a10)
> (call (mem:SI (reg:SI 56) [0 S4 A32])
> (const_int 0 [0])))
> libstdc++-v3/include/bits/atomic_base.h:287 51 {call_value_internal}
> (expr_list:REG_CALL_DECL (symbol_ref:SI ("__mulsi3") [flags 0x41])
> (expr_list:REG_EH_REGION (const_int -2147483648 [0x8000])
> (nil)))
> (expr_list (use (reg:SI 11 a11))
> (expr_list (use (reg:SI 10 a10))
> (nil
>
> vs.
>
> (insn 15 14 16 3 (set (reg:SI 57)
> (mult:SI (reg:SI 55)
> (reg:SI 56))) libstdc++-v3/include/bits/atomic_base.h:287 5
> {mulsi3}
> (nil))
>
> How can we have a mulsi3 pattern that don't get expanded until it's
> optimized, and only gets expanded to a call if it couldn't be optimized?
I'm not completely sure, but I think you want to use OPTAB_LIB as you
described above, and eliminate the TARGET_MUL32 condition in the
pattern.
Re: xtensa PR65730
On Fri, Apr 10, 2015 at 12:15 PM, Max Filippov wrote: > Then configuration w/o multiplication should call helper at -O0 and > use shift at higher optimization levels? That is what I would expect.
Re: PR65416, alloca on xtensa
On Fri, Mar 13, 2015 at 10:04 AM, Marc Gauthier wrote: > Other than the required 16-byte stack alignment, there's nothing in > the ABI that requires these extra 16 bytes. Perhaps there was a bad > implementation of the alloca exception handler at some point a long > time ago that prompted the extra 16 bytes? The alloca handler has been rewritten at least twice since this code was last updated, so that wouldn't surprise me at all. I would approve a change to eliminate it.
Re: PR65416, alloca on xtensa
On Fri, Mar 13, 2015 at 7:54 AM, Max Filippov wrote: > 1. in windowed ABI stack pointer update is always split into two opcodes: > add and movsp. How gcc optimization passes are supposed to know that > 'movsp' is related to 'add' and that stack allocation is complete only after > movsp? The movsp has a data dependency on the add, so that is not the problem. The real problem (which you identify) is that the optimizer isn't recognizing that the stack allocation isn't complete until the movsp is done. In the bug, the optimizer recognizes that a3 = sp + 32, and so it uses a3 before it updates sp to get a better schedule, but doesn't realize that the stack space hasn't been allocated yet. It looks to me like the optimizer has finally gotten smart enough that we need to implement one of the save_stack_* patterns instead of what we have now. > 2. alloca seems to make an additional 16-bytes padding to each stack > allocation: alloca(1) results in moving sp down by 32 bytes, alloca(17) > moves it by 48 bytes, etc. This padding looks unnecessary to me: either > this space is not used (previous register frame is not spilled), or alloca > exception handler will take care about reloading or moving spilled registers > to a new location. In both cases after movsp this space is just wasted. > Do you know why this padding may be needed? Answering this question definitively requires some time with the ABI manual, which I don't have. You may be right, but I would check what XCC does in this case. It is far better tested.
