This is the mail archive of the ecos-bugs@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug 1001524] Cortex-M: Remote 'g' packet reply is too long


Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001524

--- Comment #19 from Jonathan Larmour <jifl@ecoscentric.com> 2012-04-12 18:44:17 BST ---
(In reply to comment #18)
> (In reply to comment #15)
> > We need [stub_format_registers()] to skip
> > registers (ditto stub_update_registers()). We will probably need to add a hook
> > in generic-stub.c for this which cortexm_stub.h can define. Perhaps something
> > like: 
> > 
> > #ifdef TARGET_G_PACKET_SKIPS_REGS
> >     if ( !reg_in_g_packet( reg ) )
> >         continue;
> > #endif
> 
> This is a variation (Bug 1001559). Works for me and looks like solution.

Setting REGSIZE for them to 0 is good thinking.


> > Have you set HAL_STUB_REGISTERS_SIZE for example? And if so, to
> > what?
> 
> Thank you for the lead. (Bug 1001558)

Glad I guessed right :-).


> > > Ideally the variant/platform will only need to implement
> > > CYGINT_HAL_CORTEXM_FPU.
> > 
> > We can probably anticipate that other Cortex-Ms will come out in the future
> > with different VFP versions, or a different number or size of registers. 
> 
> My point was supposed to be: "In ideal case you don't have to add anything to
> variant other than implementing the interface in order to make it fit for FPU".
> In reality you need to add appropriate CFLAGS to the platform.

Not necessarily. We potentially want to treat it like ARM's thumb mode, and
have a higher layer manage adding/removing the appropriate flags to avoid
unnecessary duplication of CDL into multiple platforms.

This is also because the user has ultimate control over whether the FPU support
is included or not. Just because the hardware has an FPU doesn't mean they
definitely want to use it. After all, it will use up space for the registers
(even if they are only used lazily). And indeed your CDL below does allow it to
be enabled/disabled, which is good, but rather than every relevant platform's
CFLAGS having to test the value to decide how to construct the CFLAGS, arguably
it's better for this option to use a 'requires' to set it in the CFLAGS, e.g.
with the CDL you provided below (not my alternative):

   requires { (CYGHWR_HAL_CORTEXM_FPU == "FPV4_SP_D16") implies
is_substr(CYGBLD_GLOBAL_CFLAGS, " -mfpu=fpv4-sp-d16") }
   requires { !is_enabled(CYGHWR_HAL_CORTEXM_FPU) implies
!is_substr(CYGBLD_GLOBAL_CFLAGS, "-mfpu=fpv4-sp-d16") }


> > This should be anticipated. So it's unlikely it will be just be a question of > FPU or
> > not FPU. Maybe we should follow the example of GCC and have something like
> > CYGINT_HAL_CORTEXM_VFP4_SP_D16? Or maybe just CYGINT_HAL_CORTEXM_VFP4, and then
> > variants must use a CDL 'requires' to set a specific FPU implementation, such
> > as SP-D16.
> 
> I have changed CYGHWR_HAL_CORTEXM_FPU to booldata so now it can select desired
> FPU.
> 
>     cdl_component CYGHWR_HAL_CORTEXM_FPU {
>         display          "Floating Point Unit"
>         flavor           booldata
>         active_if        { CYGINT_HAL_CORTEXM_FPU }
>         legal_values     { "FPV4_SP_D16" }
>         default_value    { "FPV4_SP_D16" }
>         description      "Floating Point Unit"

This doesn't feel quite right. Somewhere, something will have to do both:
 implements CYGINT_HAL_CORTEXM_FPU
_and_
 requires { is_enabled(CYGHWR_HAL_CORTEXM_FPU) implies (CYGHWR_HAL_CORTEXM_FPU
== "FPV4_SP_D16" }

This seems redundant.

For that matter, is there anything to be gained from CYGINT_HAL_CORTEXM_FPU
anyway? I would have thought the HAL code would basically always have to be
customised according to the specific register layout anyway, but you are better
placed to decide that than I am.

I think it would be better to have something like:

   cdl_component CYGPKG_HAL_CORTEXM_FPU {
       display    "Hardware floating point"
       flavor     none
       no_define

       cdl_interface CYGINT_HAL_CORTEXM_FPU {
           display   "FPU present"
       }

       cdl_interface CYGINT_HAL_CORTEXM_VFP4_SP_D16 {
           display    "FPU uses vfp4-sp-d16 layout"
           implements CYGINT_HAL_CORTEXM_FPU
       }

       cdl_option CYGHWR_HAL_CORTEXM_FPU {
           display   "Use hardware FP"
           active_if CYGINT_HAL_CORTEXM_FPU
       }

       requires   { 1 >= CYGINT_HAL_CORTEXM_FPU }
       requires   { (CYGHWR_HAL_CORTEXM_FPU && CYGINT_HAL_CORTEXM_VFP4_SP_D16)
== is_substr(CYGBLD_GLOBAL_CFLAGS, " -mfpu=fpv4-sp-d16") }


(With appropriate "description" fields added of course).

In the last line, hopefully the == will make the CDL inference engine do the
right thing, but if not, change it to:
       requires   { (CYGHWR_HAL_CORTEXM_FPU && CYGINT_HAL_CORTEXM_VFP4_SP_D16)
implies is_substr(CYGBLD_GLOBAL_CFLAGS, " -mfpu=fpv4-sp-d16") }
       requires   { !(CYGHWR_HAL_CORTEXM_FPU && CYGINT_HAL_CORTEXM_VFP4_SP_D16)
implies !is_substr(CYGBLD_GLOBAL_CFLAGS, " -mfpu=fpv4-sp-d16") }


This way, HALs only need to implement CYGINT_HAL_CORTEXM_VFP4_SP_D16, and
everything else is handled here at the arch HAL level, and presented to the
user all in one place.

What do you think?


> cdl_option CYGHWR_HAL_CORTEXM_FPU_SWITCH {
>             display "FPU context switch"
>             flavor data
>             legal_values { "ALL" "LAZY" "NO" }
>             default_value { "LAZY" }
>          ....
>     }

I'd say "NONE" instead of "NO".

Jifl

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]