From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15908 invoked by alias); 17 Dec 2012 15:30:52 -0000 Received: (qmail 15874 invoked by uid 22791); 17 Dec 2012 15:30:50 -0000 X-SWARE-Spam-Status: No, hits=-2.7 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from hagrid.ecoscentric.com (HELO mail.ecoscentric.com) (212.13.207.197) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 17 Dec 2012 15:30:43 +0000 Received: from localhost (hagrid.ecoscentric.com [127.0.0.1]) by mail.ecoscentric.com (Postfix) with ESMTP id 9DD914680008 for ; Mon, 17 Dec 2012 15:30:41 +0000 (GMT) Received: from mail.ecoscentric.com ([127.0.0.1]) by localhost (hagrid.ecoscentric.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id RtdkVjDKE0zW; Mon, 17 Dec 2012 15:30:38 +0000 (GMT) From: bugzilla-daemon@bugs.ecos.sourceware.org To: ecos-patches@ecos.sourceware.org Subject: [Bug 1001607] Cortex-M4F architectural Floating Point Support X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: eCos X-Bugzilla-Component: Patches and contributions X-Bugzilla-Keywords: X-Bugzilla-Severity: enhancement X-Bugzilla-Who: ilijak@siva.com.mk X-Bugzilla-Status: NEEDINFO X-Bugzilla-Priority: low X-Bugzilla-Assigned-To: jifl@ecoscentric.com X-Bugzilla-Target-Milestone: --- X-Bugzilla-Changed-Fields: In-Reply-To: References: X-Bugzilla-URL: http://bugs.ecos.sourceware.org/ Auto-Submitted: auto-generated Content-Type: text/plain; charset="UTF-8" MIME-Version: 1.0 Date: Mon, 17 Dec 2012 15:30:00 -0000 Message-Id: <20121217153038.408EC4680002@mail.ecoscentric.com> Mailing-List: contact ecos-patches-help@ecos.sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: ecos-patches-owner@ecos.sourceware.org X-SW-Source: 2012-12/txt/msg00014.txt.bz2 Please do not reply to this email. Use the web interface provided at: http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001607 --- Comment #43 from Ilija Kocho 2012-12-17 15:30:36 GMT --- (In reply to comment #41) > (In reply to comment #40) [snip] > > > > Incidentally, I think CYGARC_CORTEXM_FPU_EXC_AUTOSAVE should default to > > > enabled, partly because of that link you posted in comment #32: > > > http://gcc.gnu.org/ml/gcc-help/2012-10/msg00056.html which means once the FPU > > > support is enabled, GCC may start generating code which uses the FPU registers; > > > > Fortunately this has been resolved in non issue after comment 32. And as you > > have commented just below (in reply to comment 27). > > Are you referring to my comment about it not being a problem that GCC may > generate code using the FPU regs for non-FP code? Being able to disable with a > compiler flag is fine for people definitely not using FP at all. However in the > above issue, my concern would be in applications which more generally _are_ > using the FPU (so have enabled CYGHWR_HAL_CORTEXM_FPU), and so they should be > able to still use the compiler optimisation in general, which means the > compiler may use FPU registers in code which has nothing to do with FP. I am referring to Joey Ye's statement that GCC does not use VFP registers for non-FP code: http://gcc.gnu.org/ml/gcc-help/2012-10/msg00056.html and that once added, such optimization shall be accompanied with flag that disables it. In such future we shall add (-fno-fpreg-for-nonfpdata) to CFLAGS. [snip, replied in comment 42] > > > > As I have a feeling that we are approaching our goal I would like to do some > > > > shaping work. One question is where is best place to put the tests. They stem > > > > from kernel tests, should I place them there or under Cortex-M architecture? > > > > > > Definitely Cortex-M. Then the CDL which provides the list of tests > > > (CYGPKG_HAL_CORTEXM_TESTS) can also only build them if CYGPKG_KERNEL is present > > > and CYGHWR_HAL_CORTEXM_FPU on (just in case, you can see examples of how to > > > create the list of test names around the place in eCos, e.g. libc stdio). > > > > > > The tests themselves may also need fine-tuning of more detailed configuration > > > options and can use CYG_TEST_NA for that if they are not applicable for the > > > configuration after all. > > > > I think they should be applicable to all configurations that have enough > > memory. > > Well... they all unconditionally include CYGPKG_KERNEL specific headers, so > must only be built if CYGPKG_KERNEL is enabled (which was my point). > > But there are other issues with the tests, especially if placed outside of the > cortex-M HAL:- > - fpinttest.c includes a call to CYGARC_MRS(). > > - thread_switch_fpu.cxx's HAL_CLOCK_READ_NS is cortex-m specific I was not clear enough, sorry. I haven't stated that I have agreed for tests to live in Cortex-M domain. My statement "applicable to all configurations ..." is in that context. > > - There are dependencies on strlen(), strcat(), fabs(), strcpy() at least - if > you want to use the compiler builtins, use e.g. __builtin_strlen instead). > Otherwise the functions may not be declared, or for fabs() won't > exist. I'll look at it. > > - alarm_fn calls time(), which obviously has no compiler builtin equivalent. > > - In thread_fp() in thread_switch_fpu.cxx: yes, GCC can optimise that out. It > is even allowed to do constant folding like that with no optimisation flag > (-O*). It didn't use to be the case for floating point constant, but now is (I > can't remember whether it's before or after 4.3, but it's true now). For this test it's only important for code to touch a VFP register. It does so for my builds (GCC 4.6.3 and 4.7.2 with default eCos flags), but yes we should be careful. > > - I'm not sure I see the value in fpinttest.c in addition to fptestf.c? A replacement rather than addition. fpinttest.c runs a mix of pure integer and float threads that enforces LAZY switches. It detected bugs (in LAZY context switching scheme) that have passed fptestf.c [snip] > > > > Just a couple more little comments: > > > > > > CYGHWR_HAL_FPV4_SP_D16 should be an option, not a component, and should > > > probably be a "calculated 1" bool rather than "flavor none". > > > > > > > Yes, it's nicer that way. I tried to make it look little bit more informative: > > > > cdl_option CYGHWR_HAL_FPV4_SP_D16 { > > flavor bool > > active_if { CYGINT_HAL_FPV4_SP_D16 } > > calculated { CYGINT_HAL_FPV4_SP_D16 && CYGHWR_HAL_CORTEXM_FPU } > > ... > > } > > Come to think of it, do we need CYGHWR_HAL_FPV4_SP_D16 at all? Can we not just > use CYGINT_HAL_FPV4_SP_D16 directly? And then make > CYGBLD_ARCH_CPUFLAG_FPV4SPD16 use (CYGHWR_HAL_CORTEXM_FPU && > CYGINT_HAL_FPV4_SP_D16) instead? As far as I can tell that component is the > only user of that setting. CYGHWR_HAL_FPV4_SP_D16 also controls compilation of fpv4_sp_d16.c. > > > Out of topic: [snip, thank you for explanation] > > What about adding > > > > #define CYGBLD_INLINE __attribute__((always_inline)) > > > > in infra/current/include/cyg_type.h? > > Note: Proposal of the name above instead of CYGBLD_ATTRIB_ALWAYS_INLINE is > > intentional. > > I don't think that name is wise because "inline" can already mean lots of > different things. It has different meanings in old GCC, C99, C++, and if > prefaced with 'static' or 'extern'. Calling something just 'inline' isn't > enough. > > What's your objection to CYGBLD_ATTRIB_ALWAYS_INLINE? Would > CYGBLD_ATTRIB_FORCE_INLINE be better? Or is it the length? Mainly length i guess. But I accept your arguments, in that case it could be either CYGBLD_ATTRIB_ALWAYS_INLINE (ATTRIB should go with ALWAYS to reflect original attribute name) or (shorter) CYGBLD_FORCE_INLINE Ilija -- Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.