This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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]

Re: [PATCH]Fix computation of offset in ivopt


On Tue, Sep 24, 2013 at 11:13 AM, bin.cheng <bin.cheng@arm.com> wrote:
> Hi,
> This patch fix two minor bugs when computing offset in IVOPT.
> 1) Considering below example:
> #define MAX 100
> struct tag
> {
>   int i;
>   int j;
> }
> struct tag arr[MAX]
>
> int foo (int len)
> {
>   int i = 0;
>   for (; i < len; i++)
>   {
>     access arr[i].j;
>   }
> }
>
> Without this patch, the offset computed by strip_offset_1 for address
> arr[i].j is ZERO, which is apparently not.
>
> 2) Considering below example:
> //...
>   <bb 15>:
>   KeyIndex_66 = KeyIndex_194 + 4294967295;
>   if (KeyIndex_66 != 0)
>     goto <bb 16>;
>   else
>     goto <bb 18>;
>
>   <bb 16>:
>
>   <bb 17>:
>   # KeyIndex_194 = PHI <KeyIndex_66(16), KeyIndex_180(73)>
>   _62 = KeyIndex_194 + 1073741823;
>   _63 = _62 * 4;
>   _64 = pretmp_184 + _63;
>   _65 = *_64;
>   if (_65 == 0)
>     goto <bb 15>;
>   else
>     goto <bb 71>;
> //...
>
> There are iv use and candidate like:
>
> use 1
>   address
>   in statement _65 = *_64;
>
>   at position *_64
>   type handletype *
>   base pretmp_184 + ((sizetype) KeyIndex_180 + 1073741823) * 4
>   step 4294967292
>   base object (void *) pretmp_184
>   related candidates
>
> candidate 6
>   var_before ivtmp.16
>   var_after ivtmp.16
>   incremented before use 1
>   type unsigned int
>   base (unsigned int) (pretmp_184 + (sizetype) KeyIndex_180 * 4)
>   step 4294967292
>   base object (void *) pretmp_184
> Candidate 6 is related to use 1
>
> In function get_computation_cost_at for use 1 using cand 6, ubase and cbase
> are:
> pretmp_184 + ((sizetype) KeyIndex_180 + 1073741823) * 4
> pretmp_184 + (sizetype) KeyIndex_180 * 4
>
> The cstepi computed in HOST_WIDE_INT is :  0xfffffffffffffffc, while offset
> computed in TYPE(utype) is : 0xfffffffc.  Though they both stand for value
> "-4" in different precision, statement "offset -= ratio * cstepi" returns
> 0x100000000, which is wrong.
>
> Tested on x86_64 and arm.  Is it OK?

+       field = TREE_OPERAND (expr, 1);
+       if (DECL_FIELD_BIT_OFFSET (field)
+           && cst_and_fits_in_hwi (DECL_FIELD_BIT_OFFSET (field)))
+         boffset = int_cst_value (DECL_FIELD_BIT_OFFSET (field));
+
+       tmp = component_ref_field_offset (expr);
+       if (top_compref
+           && cst_and_fits_in_hwi (tmp))
+         {
+           /* Strip the component reference completely.  */
+           op0 = TREE_OPERAND (expr, 0);
+           op0 = strip_offset_1 (op0, inside_addr, top_compref, &off0);
+           *offset = off0 + int_cst_value (tmp) + boffset / BITS_PER_UNIT;
+           return op0;
+         }

the failure paths seem mangled, that is, if cst_and_fits_in_hwi is false
for either offset part you may end up doing half accounting and not
stripping.

Btw, DECL_FIELD_BIT_OFFSET is always non-NULL.  I suggest to
rewrite to

     if (!inside_addr)
       return orig_expr;

     tmp = component_ref_field_offset (expr);
     field = TREE_OPERAND (expr, 1);
     if (top_compref
         && cst_and_fits_in_hwi (tmp)
         && cst_and_fits_in_hwi (DECL_FIELD_BIT_OFFSET (field)))
        {
          ...
        }

note that this doesn't really handle overflows correctly as

+           *offset = off0 + int_cst_value (tmp) + boffset / BITS_PER_UNIT;

may still overflow.

@@ -4133,6 +4142,9 @@ get_computation_cost_at (struct ivopts_data *data,
         bitmap_clear (*depends_on);
     }

+  /* Sign-extend offset if utype has lower precision than HOST_WIDE_INT.  */
+  offset = sext_hwi (offset, TYPE_PRECISION (utype));
+

offset is computed elsewhere in difference_cost and the bug to me seems that
it is unsigned.  sign-extending it here is odd at least (and the extension
should probably happen at sizetype precision, not that of utype).

Richard.


> Thanks.
> bin
>
>
> 2013-09-24  Bin Cheng  <bin.cheng@arm.com>
>
>         * tree-ssa-loop-ivopts.c (strip_offset_1): Count
>         DECL_FIELD_BIT_OFFSET when computing offset for COMPONENT_REF.
>         (get_computation_cost_at): Sign extend offset.


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