Re: remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
On May 16, 2012, at 03:10 , David Edelsohn wrote: Okay. revision 187581. Thanks!
Re: remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
Hello, since you touch the SPE area would you mind looking at this PR: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47856 -- Sebastian Huber, embedded brains GmbH Address : Obere Lagerstr. 30, D-82178 Puchheim, Germany Phone : +49 89 18 90 80 79-6 Fax : +49 89 18 90 80 79-9 E-Mail : sebastian.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Re: remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
Hello David, Back on this one after Alan's correction and extra testing of the patch on my side. On May 7, 2012, at 18:59 , David Edelsohn wrote: http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01667.html Yes, exactly. If we can work through the fallout, this is exactly the type of cleanup I envisioned. Attached is a slightly modified version, with a bug fix regarding the ISEL deactivation. The original patch kept the logic of the test but mistakenly just extracted out of an outer else conditional branch, so doing it for e500 targets as well. In this version, the test is moved in the default: part of the case statement introduced to isolate e500 specific operations, replacing the previous if/else construct. This version builds and regtests fine for default languages on powerpc-eabisim on an x86_64-linux host. It also bootstraps and regtests fine for languages=all,ada on powerpc-generic-linux-gnu. OK to commit ? With Kind Regards, Olivier 2012-05-15 Olivier Hainque hain...@adacore.com config/rs6000: * rs6000-opts.h (enum processor_type): Add PROCESSOR_PPC8548. * rs6000-cpus.def: Reference it for cpu=8548. * rs6000.md (cpu attribute definition): Add ppc8548. * 8540.md: indicate that the units/patterns apply to ppc8548 as well. * rs6000.c (rs6000_option_override_internal): Rename default_cpu into implicit_cpu, conveying what --with-cpu was passed at configure time. Treat implicit_cpu as have_cpu. Pick defaults for SPE related flags, check that what is queried is supported by the selected configuration. Rework the single/double_float and MASK_STRING resets to hit for all the E500 cores (854x + E500MC variants). Select the ppc8540 costs for PROCESSOR_PPC8548 as well. (rs6000_issue_rate): case CPU_PPC8548 together with CPU_PPC8540. (rs6000_use_sched_lookahead): Likewise, rewriting function as a case statement instead of a sequence of ifs. * rs6000.h (TARGET_E500): Remove. (TARGET_NO_LWSYNC): Adjust accordingly. * e500.h (TARGET_E500): Remove. (CHECK_E500_OPTIONS): Adjust accordingly. * eabispe.h (SUBSUBTARGET_OVERRIDE_OPTIONS): Remove. (TARGET_DEFAULT): Reformat definition to match the one in linuxspe.h. * linuxspe.h: Likewise. * vxworks.h: Remove bogus TARGET_E500 overrides and superfluous comments. * e500-double.h: Remove. gcc: * config.gcc (pick a default with_cpu): For powerpc*-*-*spe*, default to with_cpu=8548 if --enable-e500-double, and to 8540 otherwise. (set misc flags section): For powerpc*|rs6000*, remove inclusion of e500-double.h for --enable-e500-double. e500-2.diff Description: Binary data
Re: remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
On Tue, May 15, 2012 at 9:47 AM, Olivier Hainque hain...@adacore.com wrote: config/rs6000: * rs6000-opts.h (enum processor_type): Add PROCESSOR_PPC8548. * rs6000-cpus.def: Reference it for cpu=8548. * rs6000.md (cpu attribute definition): Add ppc8548. * 8540.md: indicate that the units/patterns apply to ppc8548 as well. * rs6000.c (rs6000_option_override_internal): Rename default_cpu into implicit_cpu, conveying what --with-cpu was passed at configure time. Treat implicit_cpu as have_cpu. Pick defaults for SPE related flags, check that what is queried is supported by the selected configuration. Rework the single/double_float and MASK_STRING resets to hit for all the E500 cores (854x + E500MC variants). Select the ppc8540 costs for PROCESSOR_PPC8548 as well. (rs6000_issue_rate): case CPU_PPC8548 together with CPU_PPC8540. (rs6000_use_sched_lookahead): Likewise, rewriting function as a case statement instead of a sequence of ifs. * rs6000.h (TARGET_E500): Remove. (TARGET_NO_LWSYNC): Adjust accordingly. * e500.h (TARGET_E500): Remove. (CHECK_E500_OPTIONS): Adjust accordingly. * eabispe.h (SUBSUBTARGET_OVERRIDE_OPTIONS): Remove. (TARGET_DEFAULT): Reformat definition to match the one in linuxspe.h. * linuxspe.h: Likewise. * vxworks.h: Remove bogus TARGET_E500 overrides and superfluous comments. * e500-double.h: Remove. gcc: * config.gcc (pick a default with_cpu): For powerpc*-*-*spe*, default to with_cpu=8548 if --enable-e500-double, and to 8540 otherwise. (set misc flags section): For powerpc*|rs6000*, remove inclusion of e500-double.h for --enable-e500-double. Okay. Thanks, David
remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
Hello, This is a followup on a discussion we had a while ago, starting with http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01543.html The original issue was the vxworks port unduly clobbering explicit SPE related options on powerpc, which it still does. The proposed patch at the time was to prevent the clobbering locally, adding guards in the vxworks.h code as there are in a few other targets already. Joseph and David suggested to address this in a more general fashion, along lines proposed in http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01667.html The attached patch is a first shot in this direction. Only lightly tested at this point, posted to get a first round of comments to validate that this corresponds to what was suggested. This indeed allows significant simplifications - cleaner configuration model, defaults control factorized, no need for e500-double.h any more. I'll be happy to do what I can to help in getting this in eventually. One piece that for sure needs updating is The TARGET_NO_LWSYNC change suggests that maybe keeping a TARGET_E500 macro would make sense after all, to convey cpu features an E500 core which happens to imply SPE support and no LWSYNC. We could have a TARGET_E500MC as well to make it clear that the latter family differs from the former. That's actually a detail in the set of changes at hand. In addition to what I understood of the suggestions made in the aforementioned thread, this patch includes a change on the choice of rs6000_cpu in options_override, so that it treats a --with-cpu passed at configure time as an implicit -mcpu in absence of an explicit one. This sounds reasonable and is useful for the more general change we're discussing. This patch allowed me to build a few configurations with as-expected results on gcc 4.7 base. The patch applies as-is on mainline. My first attempt at building there failed, with --target=powerpc-eabispe --with-cpu=8548 --enable-languages=c --disable-libada (internal compiler error on unwind-dw2.c) This failure reproduces with an unpatched tree as well, so there's something already broken in this area. I suppose I can can look into this one first. I'd appreciate feedback on the more general patch nevetheless :) (whether that's the right direction, things that you think should be done differently, indications as to how you'd like to proceed further (testing etc), ...) Thanks much in advance, Olivier 2012-05-07 Olivier Hainque hain...@adacore.com config/rs6000: * rs6000-opts.h (enum processor_type): Add PROCESSOR_PPC8548. * rs6000-cpus.def: Reference it for cpu=8548. * rs6000.md (cpu attribute definition): Add ppc8548. * 8540.md: indicate that the units/patterns apply to ppc8548 as well. * rs6000.c (rs6000_option_override_internal): Rename default_cpu into implicit_cpu, conveying what --with-cpu was passed at configure time. Treat implicit_cpu as have_cpu. Pick defaults for SPE related flags, check that what is queried is supported by the selected configuration. Rework the single/double_float and MASK_STRING resets to hit for all the E500 cores (854x + E500MC variants). Select the ppc8540 costs for PROCESSOR_PPC8548 as well. (rs6000_issue_rate): case CPU_PPC8548 together with CPU_PPC8540. (rs6000_use_sched_lookahead): Likewise, rewriting function as a case statement instead of a sequence of ifs. * rs6000.h (TARGET_E500): Remove. (TARGET_NO_LWSYNC): Adjust accordingly. * e500.h (TARGET_E500): Remove. (CHECK_E500_OPTIONS): Adjust accordingly. * eabispe.h (SUBSUBTARGET_OVERRIDE_OPTIONS): Remove. (TARGET_DEFAULT): Reformat definition to match the one in linuxspe.h. * linuxspe.h: Likewise. * vxworks.h: Remove bogus TARGET_E500 overrides and superfluous comments. * e500-double.h: Remove. gcc: * config.gcc (pick a default with_cpu): For powerpc*-*-*spe*, default to with_cpu=8548 if --enable-e500-double, and to 8540 otherwise. (set misc flags section): For powerpc*|rs6000*, remove inclusion of e500-double.h for --enable-e500-double. e500.dif Description: video/dv
Re: remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
On May 7, 2012, at 15:57 , Olivier Hainque wrote:
My first attempt at building there failed, with
--target=powerpc-eabispe --with-cpu=8548 --enable-languages=c
--disable-libada
(internal compiler error on unwind-dw2.c) This failure reproduces with
an unpatched tree as well, so there's something already broken in this area.
This appears to be a fallout of the (very nice !) prologue/epilogue
cleanup series issued last april, in particular rev 186797 corresponding to
http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01014.html (Alan cc'ed)
This piece:
(emit_frame_save): Don't handle reg+reg addressing.
introduces an assert on which we now trip compiling unwind-dw2.c for SPE
configurations. We now fall into the TARGET_SPE_ABI part of
/* Some cases that need register indexed addressing. */
gcc_checking_assert (!((TARGET_ALTIVEC_ABI ALTIVEC_VECTOR_MODE (mode))
|| (TARGET_VSX ALTIVEC_OR_VSX_VECTOR_MODE (mode))
|| (TARGET_E500_DOUBLE mode == DFmode)
|| (TARGET_SPE_ABI
SPE_VECTOR_MODE (mode)
!SPE_CONST_OFFSET_OK (offset;
in emit_frame_save while compiling uw_install_context.
The call comes from this part of rs6000_emit_prologue:
/* ??? There's no need to emit actual instructions here, but it's the
easiest way to get the frame unwind information emitted. */
if (crtl-calls_eh_return)
{
unsigned int i, regno;
for (i = 0; ; ++i)
{
regno = EH_RETURN_DATA_REGNO (i);
if (regno == INVALID_REGNUM)
break;
emit_frame_save (frame_reg_rtx, reg_mode, regno,
info-ehrd_offset + frame_off + reg_size * (int) i,
sp_off - frame_off);
}
}
The crash reproduces on this artificial reproducer:
void install (long offset, void * handler)
{
volatile int x [4096];
__builtin_eh_return (offset, handler);
}
configure --enable-languages=c --disable-libada --target=powerpc-eabispe
make ...
./cc1 ice.c
...
ice.c: In function install:
ice.c:6:1: internal compiler error: in emit_frame_save, at
config/rs6000/rs6000.c:18979
Re: remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
On Mon, 7 May 2012, Olivier Hainque wrote: Joseph and David suggested to address this in a more general fashion, along lines proposed in http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01667.html The attached patch is a first shot in this direction. Only lightly tested at this point, posted to get a first round of comments to validate that this corresponds to what was suggested. I haven't checked the details of the patch, but it does look substantially along the lines of what I think the logic should be. -- Joseph S. Myers jos...@codesourcery.com
Re: remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
On Mon, May 7, 2012 at 12:10 PM, Joseph S. Myers jos...@codesourcery.com wrote: On Mon, 7 May 2012, Olivier Hainque wrote: Joseph and David suggested to address this in a more general fashion, along lines proposed in http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01667.html The attached patch is a first shot in this direction. Only lightly tested at this point, posted to get a first round of comments to validate that this corresponds to what was suggested. I haven't checked the details of the patch, but it does look substantially along the lines of what I think the logic should be. Yes, exactly. If we can work through the fallout, this is exactly the type of cleanup I envisioned. Thanks, David
Re: remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
On May 7, 2012, at 18:59 , David Edelsohn wrote: Yes, exactly. If we can work through the fallout, this is exactly the type of cleanup I envisioned. Great :) Thanks, David Do you want a PR for the fallout ? It is not related to this patch at all. I can look into it further and make suggestions but this is part of a recent prologue/epilogue reorg with which I'm not yet too familiar. Olivier
Re: remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
On Mon, May 7, 2012 at 1:24 PM, Olivier Hainque hain...@adacore.com wrote: On May 7, 2012, at 18:59 , David Edelsohn wrote: Yes, exactly. If we can work through the fallout, this is exactly the type of cleanup I envisioned. Great :) Thanks, David Do you want a PR for the fallout ? It is not related to this patch at all. I can look into it further and make suggestions but this is part of a recent prologue/epilogue reorg with which I'm not yet too familiar. You can open a PR, but I assume that you mean fixing the problem before the patch is committed. I would like to give Alan a chance to comment and look into it. Thanks, David
Re: remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
On May 7, 2012, at 21:27 , Olivier Hainque wrote: Of course. That was actually my point in offering to open a PR, so that Alan (who did the nice recent reorg) can take it from there. He will certainly be more efficient than me in resolving this :) PR 53271
