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 <[email protected]> 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() <math.h> 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.
