Message ID | 20130425050221.4846d873@skate |
---|---|
State | Changes Requested |
Headers | show |
Hi Thomas, > (host-flex/host-bison) Thanks for the patches! I have integrated them and added version checks. I'm not 100% certain from which version onwards the dependency has been removed, but my colleague says that he didn't encounter these dependencies with GCC 4.5, so I made it depend on 4.2,4.3,4.4. > Do you have an idea for this pthread_mutex_t issue? Maybe ARC doesn't support NPTL or something? Correct, we don't have NPTL support yet. It was the initial planning to have this finished before we went out with the BuildRoot patches, but priorities shifted, and I hadn't disabled it yet. Added a patch for that. > Also, do you have a public repo to push your changes to? Since your patch series has become quite long, it would make it easier for us to test it as a whole. Our github site has a BuildRoot repository: https://github.com/foss-for-synopsys-dwc-arc-processors/buildroot I'm regularly rebasing & updating patches, and pushing them to this repo. Right now, there are two branches/tags that are relevant: * master_arc_20130425 * master_3.9_arc_20130425 The only difference between the two is that the first relies on 3.9-rc8, and the second on 3.9. So the second branch doesn't build right now, but should build after this weekend. BTW: would you like to repost the new set of patches on the mailing list, or would you rather have a look at it on github? BTW2: I noticed that GNU config went from GPLv2 to GPLv3, I'm not sure what the impact would be for BuildRoot, as it is really part of BuildRoot (not downloaded). Mischa
Dear Mischa Jonker, On Thu, 25 Apr 2013 11:01:15 +0000, Mischa Jonker wrote: > > (host-flex/host-bison) > Thanks for the patches! I have integrated them and added version > checks. I'm not 100% certain from which version onwards the > dependency has been removed, but my colleague says that he didn't > encounter these dependencies with GCC 4.5, so I made it depend on > 4.2,4.3,4.4. Ok. > > Do you have an idea for this pthread_mutex_t issue? Maybe ARC > > doesn't support NPTL or something? > Correct, we don't have NPTL support yet. It was the initial planning > to have this finished before we went out with the BuildRoot patches, > but priorities shifted, and I hadn't disabled it yet. Added a patch > for that. Good. > > Also, do you have a public repo to push your changes to? Since your > > patch series has become quite long, it would make it easier for us > > to test it as a whole. > Our github site has a BuildRoot repository: > https://github.com/foss-for-synopsys-dwc-arc-processors/buildroot > > I'm regularly rebasing & updating patches, and pushing them to this > repo. Right now, there are two branches/tags that are relevant: > * master_arc_20130425 > * master_3.9_arc_20130425 > > The only difference between the two is that the first relies on > 3.9-rc8, and the second on 3.9. So the second branch doesn't build > right now, but should build after this weekend. 3.9 has been released now, so that should solve your problem. Could you rebase your patch series on top of master, Peter has added 3.9 as a supported kernel. I tested your branch, and it built fine for me on the autobuilders, so in terms of basic buildability, we're good. > BTW: would you like to repost the new set of patches on the mailing > list, or would you rather have a look at it on github? BTW2: I > noticed that GNU config went from GPLv2 to GPLv3, I'm not sure what > the impact would be for BuildRoot, as it is really part of BuildRoot > (not downloaded). Yes, they should be reposted, but I have a number of comments on them: * 'toolchain/gcc: Add host-{flex,bison} dependencies for GCC 4.2,4.3,4.4' should be earlier in the series, i.e before 'arc: add gcc for ARC'. * 'package/binutils: add host-{flex,bison} dependencies for 2.19-arc' should probably be squashed into 'arc: Add support for ARC-specific binutils'. * I am not entirely happy with 'toolchain/gcc: Introduce BR2_ARCH_HAS_NO_GCC_x_y' using a reverted logic. I haven't thought too much about it, but wouldn't it be possible to instead do something like BR2_ARCH_NEEDS_GCC_4_6_PLUS, and then do something like depends !BR2_ARCH_NEEDS_GCC_4_6_PLUS on all gcc versions < 4.6 ? Hum, the problem is that it works with architecture that are supported starting from a given upstream gcc version, but not architecture that are supported only in 4.2-avr, or in 4.4-arc, etc. Maybe others will have a suggestion on this point? Other than that, it looks good to me. Thomas
On 29/04/13 22:45, Thomas Petazzoni wrote: > * I am not entirely happy with 'toolchain/gcc: Introduce > BR2_ARCH_HAS_NO_GCC_x_y' using a reverted logic. I haven't thought > too much about it, but wouldn't it be possible to instead do > something like BR2_ARCH_NEEDS_GCC_4_6_PLUS, and then do something > like depends !BR2_ARCH_NEEDS_GCC_4_6_PLUS on all gcc versions < > 4.6 ? Hum, the problem is that it works with architecture that are > supported starting from a given upstream gcc version, but not > architecture that are supported only in 4.2-avr, or in 4.4-arc, etc. > > Maybe others will have a suggestion on this point? Not entirely clean either, but what about the following: [toolchain/gcc/Config.in] config BR2_ARCH_HAS_ALL_GCC bool config BR2_ARCH_HAS_GCC_4_4 bool default y if BR2_ARCH_HAS_ALL_GCC [arch/Config.in] config BR2_mips select BR2_ARCH_HAS_ALL_GCC ... [arch/Config.in.arm] config BR2_arm1176jzf_s select BR2_ARCH_HAS_ALL_GCC ... config BR2_cortex_a8 select BR2_ARCH_HAS_GCC_4_4 ... Regards, Arnout
Dear Arnout Vandecappelle, On Tue, 30 Apr 2013 18:06:27 +0200, Arnout Vandecappelle wrote: > On 29/04/13 22:45, Thomas Petazzoni wrote: > > * I am not entirely happy with 'toolchain/gcc: Introduce > > BR2_ARCH_HAS_NO_GCC_x_y' using a reverted logic. I haven't thought > > too much about it, but wouldn't it be possible to instead do > > something like BR2_ARCH_NEEDS_GCC_4_6_PLUS, and then do something > > like depends !BR2_ARCH_NEEDS_GCC_4_6_PLUS on all gcc versions < > > 4.6 ? Hum, the problem is that it works with architecture that are > > supported starting from a given upstream gcc version, but not > > architecture that are supported only in 4.2-avr, or in 4.4-arc, etc. > > > > Maybe others will have a suggestion on this point? > > Not entirely clean either, but what about the following: > > [toolchain/gcc/Config.in] > config BR2_ARCH_HAS_ALL_GCC > bool > > config BR2_ARCH_HAS_GCC_4_4 > bool > default y if BR2_ARCH_HAS_ALL_GCC > > [arch/Config.in] > config BR2_mips > select BR2_ARCH_HAS_ALL_GCC > ... > > [arch/Config.in.arm] > config BR2_arm1176jzf_s > select BR2_ARCH_HAS_ALL_GCC > ... > config BR2_cortex_a8 > select BR2_ARCH_HAS_GCC_4_4 > ... Could be good. That said, this problem is quite different from doing the ARC port. So maybe we could instead tell Mischa to revert back to its original solution (adding yet another set of !BR2_arc everywhere), and we can work on a solution to improve this as a follow-up patch. Of course, if Mischa is willing to work on this, that'd be great, but I don't think it's really mandatory to make this cleanup a strict requirement to get the ARC code merged. The existing set of 'depends on' for gcc versions is already ugly, it's not specifically the ARC stuff that makes it ugly. Thomas
> > Maybe others will have a suggestion on this point? > > Not entirely clean either, but what about the following: > > [toolchain/gcc/Config.in] > config BR2_ARCH_HAS_ALL_GCC > bool > > config BR2_ARCH_HAS_GCC_4_4 > bool > default y if BR2_ARCH_HAS_ALL_GCC > > [arch/Config.in] > config BR2_mips > select BR2_ARCH_HAS_ALL_GCC > ... > > [arch/Config.in.arm] > config BR2_arm1176jzf_s > select BR2_ARCH_HAS_ALL_GCC > ... > config BR2_cortex_a8 > select BR2_ARCH_HAS_GCC_4_4 > ... The problem with this is that you end up in Config.in.arm with a lot of "select BR2_ARCH_HAS_ALL_GCC" lines. It is more logical, but it adds a lot of clutter IMHO. Unfortunately, we cannot make "BR2_ARCH_HAS_ALL_GCC" default y, because we don't have the 'deselect' keyword, so we need to add it everywhere. For ARM and x86, the architectures that aren't supported by all GCC versions are the exception, so instead of adding additional lines for just the exceptions, we end up with extra code everywhere. I also tried to let default values depend on earlier versions: config BR2_ARCH_HAS_GCC_4_5 bool default y if BR2_ARCH_HAS_GCC_4_4 But this doesn't work for several architectures, such as BR2_sparc_sparcsfleon, that do have GCC 4.4, but no 4.5 for instance. The reverse logic helps in the sense that the BR2_HAS_NO_GCC_x_y lines will go away over time when old GCC versions get deprecated. I will send both variants later today. Mischa
Dear Mischa Jonker, On Thu, 2 May 2013 14:03:53 +0000, Mischa Jonker wrote: [...] > But this doesn't work for several architectures, such as > BR2_sparc_sparcsfleon, that do have GCC 4.4, but no 4.5 for instance. > > The reverse logic helps in the sense that the BR2_HAS_NO_GCC_x_y > lines will go away over time when old GCC versions get deprecated. > > I will send both variants later today. As I suggested in an earlier e-mail, I think you should just not care about this problem for now, and add the multiple 'depends on !BR2_arc' as you were doing in your first version of the patch set. Your patch set already has a good number of patches, so let's merge it, and improve on top of that. Thanks! Thomas
Hi Thomas, >> I will send both variants later today. > As I suggested in an earlier e-mail, I think you should just not care about > this problem for now, and add the multiple 'depends on !BR2_arc' > as you were doing in your first version of the patch set. Your patch set > already has a good number of patches, so let's merge it, and improve on > top of that. The v3 patch set does exactly that if I'm not mistaken. This is supposed to be the improvement part already:-) Mischa
Dear Mischa Jonker, On Thu, 2 May 2013 15:18:50 +0000, Mischa Jonker wrote: > > As I suggested in an earlier e-mail, I think you should just not care about > > this problem for now, and add the multiple 'depends on !BR2_arc' > > as you were doing in your first version of the patch set. Your patch set > > already has a good number of patches, so let's merge it, and improve on > > top of that. > > The v3 patch set does exactly that if I'm not mistaken. This is supposed to be > the improvement part already:-) Yeah, seen that. I replied before I saw your v3, and the gcc patches that come after that. I'll focus on the ARC stuff for now. Thanks! Thomas
On 02/05/13 16:03, Mischa Jonker wrote: >>> Maybe others will have a suggestion on this point? >> >> Not entirely clean either, but what about the following: >> >> [toolchain/gcc/Config.in] >> config BR2_ARCH_HAS_ALL_GCC >> bool >> >> config BR2_ARCH_HAS_GCC_4_4 >> bool >> default y if BR2_ARCH_HAS_ALL_GCC >> >> [arch/Config.in] >> config BR2_mips >> select BR2_ARCH_HAS_ALL_GCC >> ... >> >> [arch/Config.in.arm] >> config BR2_arm1176jzf_s >> select BR2_ARCH_HAS_ALL_GCC >> ... >> config BR2_cortex_a8 >> select BR2_ARCH_HAS_GCC_4_4 >> ... > > The problem with this is that you end up in Config.in.arm with a lot of "select BR2_ARCH_HAS_ALL_GCC" lines. It is more logical, but it adds a lot of clutter IMHO. Okay, yet another idea. I don't know if this will actually work. [toolchain/gcc/Config.in] config BR2_ARCH_HAS_ALL_GCC bool default y if !(BR2_ARCH_HAS_GCC_4_4 || BR2_ARCH_HAS_GCC_4_5 ...) config BR2_ARCH_HAS_GCC_4_4 bool config BR2_GCC_VERSION_4_4_X depends on BR2_ARCH_HAS_ALL_GCC || BR2_ARCH_HAS_GCC_4_4 > Unfortunately, we cannot make "BR2_ARCH_HAS_ALL_GCC" default y, because we don't have the 'deselect' keyword, so we need to add it everywhere. For ARM and x86, the architectures that aren't supported by all GCC versions are the exception, so instead of adding additional lines for just the exceptions, we end up with extra code everywhere. > > I also tried to let default values depend on earlier versions: > config BR2_ARCH_HAS_GCC_4_5 > bool > default y if BR2_ARCH_HAS_GCC_4_4 > > But this doesn't work for several architectures, such as BR2_sparc_sparcsfleon, that do have GCC 4.4, but no 4.5 for instance. > > The reverse logic helps in the sense that the BR2_HAS_NO_GCC_x_y lines will go away over time when old GCC versions get deprecated. Yes, looking at the alternatives, the negative logic isn't that bad... However, the advantage of the positive logic is that it is slightly more future-safe. When adding a new gcc version, you have to remember to also add all the negative cases. Of course, you also have to remember to add a new positive case, but it is less bad if you forget that. > > I will send both variants later today. As Thomas said, don't feel obliged to continue working on this refactoring. Regards, Arnout > Mischa > >
diff --git a/toolchain/gcc/gcc-uclibc-4.x.mk b/toolchain/gcc/gcc-uclibc-4.x.mk index f4dbcae..161f33e 100644 --- a/toolchain/gcc/gcc-uclibc-4.x.mk +++ b/toolchain/gcc/gcc-uclibc-4.x.mk @@ -175,6 +175,8 @@ endif GCC_HOST_PREREQ += host-mpc endif +GCC_HOST_PREREQ = host-flex host-bison + ifeq ($(BR2_GCC_SHARED_LIBGCC),y) GCC_SHARED_LIBGCC:=--enable-shared else @@ -294,16 +296,16 @@ $(GCC_BUILD_DIR1)/.configured: $(GCC_DIR)/.patched $(GCC_BUILD_DIR1)/.compiled: $(GCC_BUILD_DIR1)/.configured $(Q)$(call MESSAGE,"Building gcc pass-1") ifeq ($(BR2_GCC_SUPPORTS_FINEGRAINEDMTUNE),y) - $(GCC_CONF_ENV) $(MAKE) -C $(GCC_BUILD_DIR1) all-gcc + $(GCC_CONF_ENV) $(HOST_MAKE_ENV) $(MAKE) -C $(GCC_BUILD_DIR1) all-gcc else - $(MAKE) -C $(GCC_BUILD_DIR1) all-gcc + $(HOST_MAKE_ENV) $(MAKE) -C $(GCC_BUILD_DIR1) all-gcc endif touch $@ gcc_initial=$(GCC_BUILD_DIR1)/.installed $(gcc_initial) $(HOST_DIR)/usr/bin/$(GNU_TARGET_NAME)-gcc: $(GCC_BUILD_DIR1)/.compiled $(Q)$(call MESSAGE,"Installing gcc pass-1") - PATH=$(TARGET_PATH) $(MAKE) -C $(GCC_BUILD_DIR1) install-gcc + $(HOST_MAKE_ENV) $(MAKE) -C $(GCC_BUILD_DIR1) install-gcc touch $(gcc_initial) gcc_initial: $(GCC_HOST_PREREQ) host-binutils $(HOST_DIR)/usr/bin/$(GNU_TARGET_NAME)-gcc @@ -365,9 +367,9 @@ $(GCC_BUILD_DIR2)/.compiled: $(GCC_BUILD_DIR2)/.configured $(Q)$(call MESSAGE,"Building gcc pass-2") # gcc >= 4.3.0 have to also build all-target-libgcc ifeq ($(BR2_GCC_SUPPORTS_FINEGRAINEDMTUNE),y) - $(GCC_CONF_ENV) $(MAKE) -C $(GCC_BUILD_DIR2) all-gcc all-target-libgcc + $(GCC_CONF_ENV) $(HOST_MAKE_ENV) $(MAKE) -C $(GCC_BUILD_DIR2) all-gcc all-target-libgcc else - $(MAKE) -C $(GCC_BUILD_DIR2) all-gcc + $(HOST_MAKE_ENV) $(MAKE) -C $(GCC_BUILD_DIR2) all-gcc endif touch $@ @@ -376,9 +378,9 @@ $(gcc_intermediate): $(GCC_BUILD_DIR2)/.compiled $(Q)$(call MESSAGE,"Installing gcc pass-2") # gcc >= 4.3.0 have to also install install-target-libgcc ifeq ($(BR2_GCC_SUPPORTS_FINEGRAINEDMTUNE),y) - PATH=$(TARGET_PATH) $(MAKE) -C $(GCC_BUILD_DIR2) install-gcc install-target-libgcc + $(HOST_MAKE_ENV) $(MAKE) -C $(GCC_BUILD_DIR2) install-gcc install-target-libgcc else - PATH=$(TARGET_PATH) $(MAKE) -C $(GCC_BUILD_DIR2) install-gcc + $(HOST_MAKE_ENV) $(MAKE) -C $(GCC_BUILD_DIR2) install-gcc endif touch $(gcc_intermediate) @@ -444,12 +446,12 @@ $(GCC_BUILD_DIR3)/.configured: $(GCC_SRC_DIR)/.patched $(GCC_STAGING_PREREQ) $(GCC_BUILD_DIR3)/.compiled: $(GCC_BUILD_DIR3)/.configured $(Q)$(call MESSAGE,"Building gcc final") - $(GCC_CONF_ENV) $(MAKE) -C $(GCC_BUILD_DIR3) all + $(GCC_CONF_ENV) $(HOST_MAKE_ENV) $(MAKE) -C $(GCC_BUILD_DIR3) all touch $@ $(GCC_BUILD_DIR3)/.installed: $(GCC_BUILD_DIR3)/.compiled $(Q)$(call MESSAGE,"Installing gcc final") - PATH=$(TARGET_PATH) $(MAKE) \ + $(HOST_MAKE_ENV) $(MAKE) \ -C $(GCC_BUILD_DIR3) install if [ -d "$(STAGING_DIR)/lib64" ]; then \ if [ ! -e "$(STAGING_DIR)/lib" ]; then \ @@ -577,8 +579,8 @@ $(GCC_BUILD_DIR4)/.configured: $(GCC_BUILD_DIR4)/.prepared $(GCC_BUILD_DIR4)/.compiled: $(GCC_BUILD_DIR4)/.configured $(Q)$(call MESSAGE,"Building gcc on target") - PATH=$(TARGET_PATH) \ - $(MAKE) -C $(GCC_BUILD_DIR4) all + $(TARGET_MAKE_ENV) \ + $(TARGET_MAKE_ENV) $(MAKE) -C $(GCC_BUILD_DIR4) all touch $@ GCC_LIB_SUBDIR=lib/gcc/$(GNU_TARGET_NAME)/$(GCC_VERSION) @@ -590,7 +592,7 @@ endif $(TARGET_DIR)/usr/bin/gcc: $(GCC_BUILD_DIR4)/.compiled $(Q)$(call MESSAGE,"Installing gcc on target") - PATH=$(TARGET_PATH) DESTDIR=$(TARGET_DIR) \ + $(TARGET_MAKE_ENV) DESTDIR=$(TARGET_DIR) \ $(MAKE1) -C $(GCC_BUILD_DIR4) install # Remove broken specs file (cross compile flag is set). rm -f $(TARGET_DIR)/usr/$(GCC_LIB_SUBDIR)/specs