IMGJulian commented on a change in pull request #474: PIC32: Fix stack align
URL: https://github.com/apache/mynewt-core/pull/474#discussion_r131878603
 
 

 ##########
 File path: kernel/os/include/os/arch/pic32/os/os_arch.h
 ##########
 @@ -38,7 +38,8 @@ typedef uint32_t os_sr_t;
 /* Stack type, aligned to a 32-bit word. */
 #define OS_STACK_PATTERN    (0xdeadbeef)
 
-typedef uint32_t os_stack_t;
+/* uint64_t in an attempt to get the stack 8 aligned */
+typedef uint64_t os_stack_t;
 
 Review comment:
   That makes a lot of sense. I have just dug out my old attempt at that code 
and remembered another problem I had with it: I am storing my lazy FPU context 
as an offset to the top of the stack (which must be aligned). The actual top of 
the stack cannot be modified in the `os_arch_task_stack_init()` function as it 
is overwritten shortly after. Do you know how much would be broken by changing 
this (in os_task.c):
   
   ```
   t->t_stackptr = os_arch_task_stack_init(t, &stack_bottom[stack_size],
               stack_size);
   t->t_stacktop = &stack_bottom[stack_size];
   t->t_stacksize = stack_size;
   ```
   
   to this:
   
   ```
   t->t_stacktop = &stack_bottom[stack_size];
   t->t_stacksize = stack_size;
   t->t_stackptr = os_arch_task_stack_init(t, &stack_bottom[stack_size],
               stack_size);
   ```
   ? (You could even get rid of 2 args to `os_arch_task_stack_init()`).
   
   The alternative is computing the alignment every time I want to access the 
FPU context.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to