Dale Johannesen <[EMAIL PROTECTED]> writes:

> The test of f->b comes out as
> 
>    testl  $1048512, 73(%eax)
> 
> This is wrong, because 4 bytes starting at 73 goes outside the
> original object and can
> cause a page fault.  The change from referencing a word at offset 72
> to offset 73
> happens in make_extraction in combine, and I propose to fix it thus:
> 
> 
> Index: combine.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/combine.c,v
> retrieving revision 1.502
> diff -u -b -c -3 -p -r1.502 combine.c
> cvs diff: conflicting specifications of output style
> *** combine.c 8 Aug 2005 18:30:09 -0000       1.502
> --- combine.c 25 Aug 2005 17:57:21 -0000
> *************** make_extraction (enum machine_mode mode,
> *** 6484,6491 ****
>         && GET_MODE_SIZE (inner_mode) < GET_MODE_SIZE (is_mode))
>       offset -= GET_MODE_SIZE (is_mode) - GET_MODE_SIZE (inner_mode);
>   
> !       /* If this is a constant position, we can move to the desired byte.  
> */
> !       if (pos_rtx == 0)
>       {
>         offset += pos / BITS_PER_UNIT;
>         pos %= GET_MODE_BITSIZE (wanted_inner_mode);
> --- 6484,6493 ----
>         && GET_MODE_SIZE (inner_mode) < GET_MODE_SIZE (is_mode))
>       offset -= GET_MODE_SIZE (is_mode) - GET_MODE_SIZE (inner_mode);
>   
> !       /* If this is a constant position, we can move to the desired byte.
> !      This is unsafe for memory objects; it might result in accesses
> !      outside the original object.  */
> !       if (pos_rtx == 0 && !MEM_P (inner))
>       {
>         offset += pos / BITS_PER_UNIT;
>         pos %= GET_MODE_BITSIZE (wanted_inner_mode);
> 
> 
> Still testing, but I'm a bit concerned this is overkill.  Are there
> targets/situations where
> this transformation is useful or even necessary?  Comments?

This optimization is going to be desirable when testing a byte value,
especially on a target which can test a byte but not a long, like the
m68k.

However, it's odd that we are doing this when we are extracting a
value that is larger than a byte.  And the code looks wrong if the
conditional is true--I would have thought that both offset and pos
should use BITS_PER_UNIT, or both should use GET_MODE_BITSIZE
(wanted_inner_mode).

I think something like this might be more reasonable:

    if (pos_rtx == 0)
      {
        enum machine_mode bfmode = smallest_mode_for_size (len, MODE_INT);
        offset += pos / GET_MODE_BITSIZE (bfmode);
        pos %= GET_MODE_BITSIZE (bfmode);
      }

Ian

Reply via email to