On 08/11/16, Richard Biener wrote:
>
> The patch looks mostly ok, but
>
> + else
> + {
> + boff >>= LOG2_BITS_PER_UNIT;
> + boff += tree_to_uhwi (component_ref_field_offset (ref));
> + coff = size_binop (MINUS_EXPR, coff, ssize_int (boff));
>
> how can we be sure that component_ref_field_offset is an INTEGER_CST?
> At least Ada can have variably-length fields before a bitfield. We'd
> need to apply component_ref_field_offset to off in that case. Which
> makes me wonder if we can simply avoid the COMPONENT_REF path in
> a first iteration of the patch and always build a BIT_FIELD_REF?
> It should solve the correctness issues as well and be more applicable
> for branches.
>
Oh yes, thanks for catching that!
If that information is true, that ought to go into the comment before
the if, that would certainly be an interesting comment :-)
Are there any test cases for this non-constant field offsets?
I see many checks if TREE_TYPE of
component_ref_field_offset is INTEGER_CST, but with very little
background why it could be otherwise.
I think we should simply fall back to the BIT_FIELD_REF in that case,
that would mean, the if should be something like:
tree offset = component_ref_field_offset (ref);
if (boff % BITS_PER_UNIT != 0
|| !tree_fits_uhwi_p (offset))
And yes, the correctness issue can certainly be solved with the
BIT_FIELD_REF alone.
So, as requested, here is a first iteration of my patch that always builds
a BIT_FIELD_REF, together with the test cases.
Boot-strap & regression testing was done on x86_64-pc-linux-gnu.
Is it OK for trunk (and active branches)?
Thanks
Bernd.2016-08-11 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR tree-optimization/71083
* tree-predcom.c (ref_at_iteration): Correctly align the
reference type.
testsuite:
2016-08-11 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR tree-optimization/71083
* gcc.c-torture/execute/pr71083.c: New test.
* gnat.dg/loop_optimization23.adb: New test.
* gnat.dg/loop_optimization23_pkg.ads: New test.
* gnat.dg/loop_optimization23_pkg.adb: New test.
Index: gcc/tree-predcom.c
===================================================================
--- gcc/tree-predcom.c (revision 239193)
+++ gcc/tree-predcom.c (working copy)
@@ -213,6 +213,7 @@ along with GCC; see the file COPYING3.
#include "tree-scalar-evolution.h"
#include "params.h"
#include "tree-affine.h"
+#include "builtins.h"
/* The maximum number of iterations between the considered memory
references. */
@@ -1381,6 +1382,8 @@ ref_at_iteration (data_reference_p dr, i
addr = force_gimple_operand_1 (unshare_expr (addr), stmts,
is_gimple_mem_ref_addr, NULL_TREE);
tree alias_ptr = fold_convert (reference_alias_ptr_type (DR_REF (dr)), coff);
+ tree type = build_aligned_type (TREE_TYPE (DR_REF (dr)),
+ get_object_alignment (DR_REF (dr)));
/* While data-ref analysis punts on bit offsets it still handles
bitfield accesses at byte boundaries. Cope with that. Note that
we cannot simply re-apply the outer COMPONENT_REF because the
@@ -1392,12 +1395,11 @@ ref_at_iteration (data_reference_p dr, i
{
tree field = TREE_OPERAND (DR_REF (dr), 1);
return build3 (BIT_FIELD_REF, TREE_TYPE (DR_REF (dr)),
- build2 (MEM_REF, DECL_BIT_FIELD_TYPE (field),
- addr, alias_ptr),
+ build2 (MEM_REF, type, addr, alias_ptr),
DECL_SIZE (field), bitsize_zero_node);
}
else
- return fold_build2 (MEM_REF, TREE_TYPE (DR_REF (dr)), addr, alias_ptr);
+ return fold_build2 (MEM_REF, type, addr, alias_ptr);
}
/* Get the initialization expression for the INDEX-th temporary variable
Index: gcc/testsuite/gcc.c-torture/execute/pr71083.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/pr71083.c (revision 0)
+++ gcc/testsuite/gcc.c-torture/execute/pr71083.c (working copy)
@@ -0,0 +1,43 @@
+struct lock_chain {
+ unsigned int irq_context: 2,
+ depth: 6,
+ base: 24;
+};
+
+__attribute__((noinline, noclone))
+struct lock_chain * foo (struct lock_chain *chain)
+{
+ int i;
+ for (i = 0; i < 100; i++)
+ {
+ chain[i+1].base = chain[i].base;
+ }
+ return chain;
+}
+
+struct lock_chain1 {
+ char x;
+ unsigned short base;
+} __attribute__((packed));
+
+__attribute__((noinline, noclone))
+struct lock_chain1 * bar (struct lock_chain1 *chain)
+{
+ int i;
+ for (i = 0; i < 100; i++)
+ {
+ chain[i+1].base = chain[i].base;
+ }
+ return chain;
+}
+
+struct lock_chain test [101];
+struct lock_chain1 test1 [101];
+
+int
+main ()
+{
+ foo (test);
+ bar (test1);
+ return 0;
+}
Index: gcc/testsuite/gnat.dg/loop_optimization23.adb
===================================================================
--- gcc/testsuite/gnat.dg/loop_optimization23.adb (revision 0)
+++ gcc/testsuite/gnat.dg/loop_optimization23.adb (working copy)
@@ -0,0 +1,14 @@
+-- { dg-do run }
+-- { dg-options "-O3" }
+-- PR tree-optimization/71083
+with Loop_Optimization23_Pkg;
+use Loop_Optimization23_Pkg;
+procedure Loop_Optimization23 is
+ Test : ArrayOfStructB;
+begin
+ Test (0).b.b := 9999;
+ Foo (Test);
+ if Test (100).b.b /= 9999 then
+ raise Program_Error;
+ end if;
+end;
Index: gcc/testsuite/gnat.dg/loop_optimization23_pkg.adb
===================================================================
--- gcc/testsuite/gnat.dg/loop_optimization23_pkg.adb (revision 0)
+++ gcc/testsuite/gnat.dg/loop_optimization23_pkg.adb (working copy)
@@ -0,0 +1,11 @@
+-- { dg-do compile }
+-- { dg-options "-O3" }
+-- PR tree-optimization/71083
+package body Loop_Optimization23_Pkg is
+ procedure Foo (X : in out ArrayOfStructB) is
+ begin
+ for K in 0..99 loop
+ X (K+1).b.b := X (K).b.b;
+ end loop;
+ end Foo;
+end Loop_Optimization23_Pkg;
Index: gcc/testsuite/gnat.dg/loop_optimization23_pkg.ads
===================================================================
--- gcc/testsuite/gnat.dg/loop_optimization23_pkg.ads (revision 0)
+++ gcc/testsuite/gnat.dg/loop_optimization23_pkg.ads (working copy)
@@ -0,0 +1,17 @@
+-- PR tree-optimization/71083
+package Loop_Optimization23_Pkg is
+ type Nibble is mod 2**4;
+ type Int24 is mod 2**24;
+ type StructA is record
+ a : Nibble;
+ b : Int24;
+ end record;
+ pragma Pack(StructA);
+ type StructB is record
+ a : Nibble;
+ b : StructA;
+ end record;
+ pragma Pack(StructB);
+ type ArrayOfStructB is array(0..100) of StructB;
+ procedure Foo (X : in out ArrayOfStructB);
+end Loop_Optimization23_Pkg;
