RFC: ARM/Xscale dependency or lifetime bug?

Daniel Jacobowitz drow@mvista.com
Thu Apr 17 21:35:00 GMT 2003


Ping?

I believe that this affects the 3.3 branch, and that the toplev.c patch
should be merged to the branch.  Note that the date in Richard
Sandiford's changelog is wrong; it was 2003-02-09.

However, the test case does not fail on the 3.3 branch, so I'm not sure
it qualifies as a regression.  I'd still appreciate the sanity check.

On Thu, Mar 27, 2003 at 04:10:50PM -0500, Daniel Jacobowitz wrote:
> I've been investigating an interesting Xscale bug on 3.2 all day.  Testcase
> attached.  It first appeared on 2001-01-03 with this patch:
> 
> @@ -16,6 +16,12 @@ Wed Jan  3 08:53:50 2001  Richard Kenner
>         * cse.c (cse_rtx_varies_p): Accept additional FROM_ALIAS arg.  All
>         callers changed.
> 
> +       * alias.c (throughout): Use ORIGINAL_REGNO when accessing
> +       reg_base_value and reg_known_value arrays.
> +       (init_alias_analysis): Add more cases to detect known values.
> +       * sched-deps.c (deps_may_trap_p): New function.
> +       (sched_analyze_2): Use it.
> +
> 
> And then it vanished again in HEAD with this patch (which wasn't applied
> until two months after it was posted):
> 
>   http://gcc.gnu.org/ml/gcc-patches/2002-12/msg00461.html
> +2002-02-09  Richard Sandiford  <rsandifo@redhat.com>
> +
> +       * toplev.c (rest_of_compilation): Recompute register usage after
> +       split_all_insns.
> 
> 
> I didn't check 3.3 yet.
> 
> The problem is that given this type:
> typedef struct _trace_buffer_startzz
> { 
>   unsigned char Size;
>   struct timeval Time;
>   unsigned long ID;
> } __attribute__ ((packed)) trace_buffer_startzz;
> 
> 
> and these statements:
>   lStartBufferEvent.Size = sizeof(lStartBufferEvent);
>   lStartBufferEvent.Time = zsBufferStartTime;
> 
> Compile using an arm-elf compiler, -mcpu=xscale -Os.
> 
> The code generated before scheduling uses a byte store for Size, then loads
> the first word of the structure, masks it, adds in the first bit of the Time
> field, and stores it.  The load then gets scheduled before the byte store
> and the value of Size is lost.
> 
> Note that disabling the xscale override in this bit of code in arm.c:
>   /* If optimizing for space, don't synthesize constants.
>      For processors with load scheduling, it never costs more than 2 cycles
>      to load a constant, and the load scheduler may well reduce that to 1.  */
>   if (optimize_size || (tune_flags & FL_LDSCHED))
>     arm_constant_limit = 1;
>   
>   if (arm_is_xscale)
>     arm_constant_limit = 2;
> 
> Also makes the problem go away.  I'm not sure why.  [As an aside, the above
> looks bogus; if we're optimizing for size on Xscale shouldn't we load
> constants from memory?]
> 
> 
> Is Richard Sandiford's patch a correct fix for this problem, or is it
> masking it?  I need some help here.
> 
> My SWAG is that the former patch caused us to recognize that the slots were
> the same (? or maybe recognize that they were different?) causing us to make
> some different decision on the faulty information in REG_N_SETS, and the
> latter patch caused REG_N_SETS to be corrected.
> 
> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
> 
> struct timeval {
>   long tv_sec, tv_usec;
> };
> 
> static struct timeval zsBufferStartTime;
> static unsigned long zsBufferID;
> 
> typedef struct _trace_buffer_startzz
> {
>   unsigned char Size;
>   struct timeval Time;
>   unsigned long ID;
> } __attribute__ ((packed)) trace_buffer_startzz;
> 
> unsigned int initialSize, resultSize;
> 
> void foo(const char *fmt, unsigned int a, unsigned int b, unsigned int c,
> 	 unsigned int d)
> {
>   initialSize = a;
> }
> 
> int main(void)
> {
>   char lNewUserEvent[512];
>   trace_buffer_startzz lStartBufferEvent;
>   int k;
> 
>   lStartBufferEvent.Size = sizeof(lStartBufferEvent);
>   lStartBufferEvent.Time = zsBufferStartTime;
> 
>   foo("ACTUAL size 0x%02x  id 0x%08x  time 0x%08x 0x%08x\n",
> 	lStartBufferEvent.Size,
> 	(unsigned int)lStartBufferEvent.ID,
> 	(unsigned int)lStartBufferEvent.Time.tv_sec,
> 	(unsigned int)lStartBufferEvent.Time.tv_usec);
> 
>   resultSize = lStartBufferEvent.Size;
> 
>   if (resultSize != initialSize)
>     abort ();
> 
>   return 0;
> }
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer



More information about the Gcc-patches mailing list