[PATCH] Fix oversight in bitfield_overlaps_p

[PATCH] Fix oversight in bitfield_overlaps_p

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 744 bytes --]

Hi,

the x86-64 compiler generates wrong code on the 4.3 branch for the attached 
testcase because the constant folder turns the test

    if Var.Length = 1
     and then Var.Content (1) = Bla

into a BIT_FIELD_REF and then SRA wrongly computes that in

      M.Length := 1;
      M.Content := (others => Bla);

the initialization of M.Content doesn't matter.  bitfield_overlaps_p simply 
considers that all arrays are zero-based, which is not often true in Ada.

Tested on x86_64-suse-linux, OK for mainline and 4.3 branch?


2008-10-31  Eric Botcazou  <ebotcazou@adacore.com>

	* tree-sra.c (bitfield_overlaps_p): Fix oversight.


2008-10-31  Eric Botcazou  <ebotcazou@adacore.com>

        * gnat.dg/array5.adb New test.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 711 bytes --]

Index: tree-sra.c
===================================================================
--- tree-sra.c	(revision 141459)
+++ tree-sra.c	(working copy)
@@ -2961,8 +2961,13 @@ bitfield_overlaps_p (tree blen, tree bpo
     }
   else if (TREE_CODE (fld->element) == INTEGER_CST)
     {
+      tree domain_type = TYPE_DOMAIN (TREE_TYPE (fld->parent->element));
       flen = fold_convert (bitsizetype, TYPE_SIZE (fld->type));
       fpos = fold_convert (bitsizetype, fld->element);
+      if (domain_type && TYPE_MIN_VALUE (domain_type))
+	fpos = size_binop (MINUS_EXPR, fpos,
+			   fold_convert (bitsizetype,
+			   		 TYPE_MIN_VALUE (domain_type)));
       fpos = size_binop (MULT_EXPR, flen, fpos);
     }
   else

[-- Attachment #3: p.adb --]
[-- Type: text/x-adasrc, Size: 570 bytes --]

-- { dg-do run }
-- { dg-options "-O" }

procedure P is

    type myint is range 0 .. 100_000;
    Bla : constant myint := 359;

    type my_array is array (1 .. 2) of myint;

    type item is record
       Length  : Integer;
       Content : my_array;
    end record;

    procedure create_item (M : out item) is
    begin
      M.Length := 1;
      M.Content := (others => Bla);
    end;

    Var : item;

begin
    create_item (Var);

    if Var.Length = 1
     and then Var.Content (1) = Bla
    then
      null;
    else
      raise Program_Error;
    end if;
end;

Re: [PATCH] Fix oversight in bitfield_overlaps_p

[Diego Novillo]

On Fri, Oct 31, 2008 at 07:27, Eric Botcazou <ebotcazou@adacore.com> wrote:

> 2008-10-31  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * tree-sra.c (bitfield_overlaps_p): Fix oversight.
>
>
> 2008-10-31  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * gnat.dg/array5.adb New test.

OK.


Diego.

Re: [PATCH] Fix oversight in bitfield_overlaps_p

[Richard Guenther]

On Fri, Oct 31, 2008 at 12:27 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> the x86-64 compiler generates wrong code on the 4.3 branch for the attached
> testcase because the constant folder turns the test
>
>    if Var.Length = 1
>     and then Var.Content (1) = Bla
>
> into a BIT_FIELD_REF and then SRA wrongly computes that in
>
>      M.Length := 1;
>      M.Content := (others => Bla);
>
> the initialization of M.Content doesn't matter.  bitfield_overlaps_p simply
> considers that all arrays are zero-based, which is not often true in Ada.
>
> Tested on x86_64-suse-linux, OK for mainline and 4.3 branch?

Does this work with variable bases, that is if the minimum value is gimplified?
I think we should use array_ref_low_bound properly, which of course requires
access to the original array-ref here.  Or does SRA punt for ARRAY_REFs
with variable lower bound?  The patch is ok in this case or with a change to
make SRA punt in this case.

Thanks,
Richard.

>
> 2008-10-31  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * tree-sra.c (bitfield_overlaps_p): Fix oversight.
>
>
> 2008-10-31  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * gnat.dg/array5.adb New test.
>
>
> --
> Eric Botcazou
>

Re: [PATCH] Fix oversight in bitfield_overlaps_p

[Eric Botcazou]

> Or does SRA punt for ARRAY_REFs with variable lower bound?

Yes, SRA punts for all variable-sized types, see sra_type_can_be_decomposed_p.

-- 
Eric Botcazou