diff mbox

Fix PR80334

Message ID alpine.LSU.2.20.1704061438350.30715@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener April 6, 2017, 12:40 p.m. UTC 
The following patch makes sure to preserve (mis-)alignment of memory
references when IVOPTs generates TARGET_MEM_REFs for them.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

2017-04-06  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80334
	* tree-ssa-loop-ivopts.c (rewrite_use_address): Properly
	preserve alignment of accesses.

	* g++.dg/torture/pr80334.C: New testcase.

Comments

Rainer Orth April 6, 2017, 8:44 p.m. UTC | #1 
Hi Richard,

> The following patch makes sure to preserve (mis-)alignment of memory
> references when IVOPTs generates TARGET_MEM_REFs for them.
>
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>
> Richard.
>
> 2017-04-06  Richard Biener  <rguenther@suse.de>
>
> 	PR tree-optimization/80334
> 	* tree-ssa-loop-ivopts.c (rewrite_use_address): Properly
> 	preserve alignment of accesses.
>
> 	* g++.dg/torture/pr80334.C: New testcase.

the new testcase FAILs on 32-bit Solaris/SPARC:

+FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)
+FAIL: g++.dg/torture/pr80334.C   -O1  (test for excess errors)
+FAIL: g++.dg/torture/pr80334.C   -O2  (test for excess errors)
+FAIL: g++.dg/torture/pr80334.C   -O2 -flto  (test for excess errors)
+FAIL: g++.dg/torture/pr80334.C   -O2 -flto -flto-partition=none  (test for exce
ss errors)
+FAIL: g++.dg/torture/pr80334.C   -O3 -fomit-frame-pointer -funroll-loops -fpeel
-loops -ftracer -finline-functions  (test for excess errors)
+FAIL: g++.dg/torture/pr80334.C   -O3 -g  (test for excess errors)
+FAIL: g++.dg/torture/pr80334.C   -Os  (test for excess errors)

Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/torture/pr80334.C:11:20: warning: requested alignment 16 is larger than 8 [-Wattributes]

	Rainer
Richard Biener April 7, 2017, 6:49 a.m. UTC | #2 
On Thu, 6 Apr 2017, Rainer Orth wrote:

> Hi Richard,
> 
> > The following patch makes sure to preserve (mis-)alignment of memory
> > references when IVOPTs generates TARGET_MEM_REFs for them.
> >
> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> >
> > Richard.
> >
> > 2017-04-06  Richard Biener  <rguenther@suse.de>
> >
> > 	PR tree-optimization/80334
> > 	* tree-ssa-loop-ivopts.c (rewrite_use_address): Properly
> > 	preserve alignment of accesses.
> >
> > 	* g++.dg/torture/pr80334.C: New testcase.
> 
> the new testcase FAILs on 32-bit Solaris/SPARC:
> 
> +FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)
> +FAIL: g++.dg/torture/pr80334.C   -O1  (test for excess errors)
> +FAIL: g++.dg/torture/pr80334.C   -O2  (test for excess errors)
> +FAIL: g++.dg/torture/pr80334.C   -O2 -flto  (test for excess errors)
> +FAIL: g++.dg/torture/pr80334.C   -O2 -flto -flto-partition=none  (test for exce
> ss errors)
> +FAIL: g++.dg/torture/pr80334.C   -O3 -fomit-frame-pointer -funroll-loops -fpeel
> -loops -ftracer -finline-functions  (test for excess errors)
> +FAIL: g++.dg/torture/pr80334.C   -O3 -g  (test for excess errors)
> +FAIL: g++.dg/torture/pr80334.C   -Os  (test for excess errors)
> 
> Excess errors:
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/torture/pr80334.C:11:20: warning: requested alignment 16 is larger than 8 [-Wattributes]

Any suggestion how to mitigate that?  Possible solution includes
adding { target { ! ... } } to dg-do run.

Richard.
Rainer Orth April 7, 2017, 8:21 a.m. UTC | #3 
Hi Richard,

> On Thu, 6 Apr 2017, Rainer Orth wrote:
>
>> Hi Richard,
>> 
>> > The following patch makes sure to preserve (mis-)alignment of memory
>> > references when IVOPTs generates TARGET_MEM_REFs for them.
>> >
>> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>> >
>> > Richard.
>> >
>> > 2017-04-06  Richard Biener  <rguenther@suse.de>
>> >
>> > 	PR tree-optimization/80334
>> > 	* tree-ssa-loop-ivopts.c (rewrite_use_address): Properly
>> > 	preserve alignment of accesses.
>> >
>> > 	* g++.dg/torture/pr80334.C: New testcase.
>> 
>> the new testcase FAILs on 32-bit Solaris/SPARC:
>> 
>> +FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)
>> +FAIL: g++.dg/torture/pr80334.C   -O1  (test for excess errors)
>> +FAIL: g++.dg/torture/pr80334.C   -O2  (test for excess errors)
>> +FAIL: g++.dg/torture/pr80334.C   -O2 -flto  (test for excess errors)
>> +FAIL: g++.dg/torture/pr80334.C -O2 -flto -flto-partition=none (test for
>> exce
>> ss errors)
>> +FAIL: g++.dg/torture/pr80334.C -O3 -fomit-frame-pointer -funroll-loops
>> -fpeel
>> -loops -ftracer -finline-functions  (test for excess errors)
>> +FAIL: g++.dg/torture/pr80334.C   -O3 -g  (test for excess errors)
>> +FAIL: g++.dg/torture/pr80334.C   -Os  (test for excess errors)
>> 
>> Excess errors:
>> /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/torture/pr80334.C:11:20:
>> warning: requested alignment 16 is larger than 8 [-Wattributes]
>
> Any suggestion how to mitigate that?  Possible solution includes
> adding { target { ! ... } } to dg-do run.

No idea.  However, according to gcc-testresults there are other
failures: s390-ibm-inux-gnu and s390x-ibm-linux-gnu so far.

This might argue against just excluding a random list of failing targets.

	Rainer
Richard Biener April 7, 2017, 8:33 a.m. UTC | #4 
On Fri, 7 Apr 2017, Rainer Orth wrote:

> Hi Richard,
> 
> > On Thu, 6 Apr 2017, Rainer Orth wrote:
> >
> >> Hi Richard,
> >> 
> >> > The following patch makes sure to preserve (mis-)alignment of memory
> >> > references when IVOPTs generates TARGET_MEM_REFs for them.
> >> >
> >> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> >> >
> >> > Richard.
> >> >
> >> > 2017-04-06  Richard Biener  <rguenther@suse.de>
> >> >
> >> > 	PR tree-optimization/80334
> >> > 	* tree-ssa-loop-ivopts.c (rewrite_use_address): Properly
> >> > 	preserve alignment of accesses.
> >> >
> >> > 	* g++.dg/torture/pr80334.C: New testcase.
> >> 
> >> the new testcase FAILs on 32-bit Solaris/SPARC:
> >> 
> >> +FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)
> >> +FAIL: g++.dg/torture/pr80334.C   -O1  (test for excess errors)
> >> +FAIL: g++.dg/torture/pr80334.C   -O2  (test for excess errors)
> >> +FAIL: g++.dg/torture/pr80334.C   -O2 -flto  (test for excess errors)
> >> +FAIL: g++.dg/torture/pr80334.C -O2 -flto -flto-partition=none (test for
> >> exce
> >> ss errors)
> >> +FAIL: g++.dg/torture/pr80334.C -O3 -fomit-frame-pointer -funroll-loops
> >> -fpeel
> >> -loops -ftracer -finline-functions  (test for excess errors)
> >> +FAIL: g++.dg/torture/pr80334.C   -O3 -g  (test for excess errors)
> >> +FAIL: g++.dg/torture/pr80334.C   -Os  (test for excess errors)
> >> 
> >> Excess errors:
> >> /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/torture/pr80334.C:11:20:
> >> warning: requested alignment 16 is larger than 8 [-Wattributes]
> >
> > Any suggestion how to mitigate that?  Possible solution includes
> > adding { target { ! ... } } to dg-do run.
> 
> No idea.  However, according to gcc-testresults there are other
> failures: s390-ibm-inux-gnu and s390x-ibm-linux-gnu so far.
> 
> This might argue against just excluding a random list of failing targets.

Hmm.  Does using __BIGGEST_ALIGNMENT__, thus

int
main()
{
  alignas(__BIGGEST_ALIGNMENT__) B b[3];
...

work for you?

Richard.
Rainer Orth April 7, 2017, 8:41 a.m. UTC | #5 
Hi Richard,

>> > Any suggestion how to mitigate that?  Possible solution includes
>> > adding { target { ! ... } } to dg-do run.
>> 
>> No idea.  However, according to gcc-testresults there are other
>> failures: s390-ibm-inux-gnu and s390x-ibm-linux-gnu so far.
>> 
>> This might argue against just excluding a random list of failing targets.
>
> Hmm.  Does using __BIGGEST_ALIGNMENT__, thus
>
> int
> main()
> {
>   alignas(__BIGGEST_ALIGNMENT__) B b[3];
> ...
>
> work for you?

it does: the test now PASSes on sparc-sun-solaris2.12 and continues to
do so in i386-pc-solaris2.12, both 32 and 64-bit.

Thanks.
        Rainer
diff mbox

Patch

Index: gcc/tree-ssa-loop-ivopts.c
===================================================================
--- gcc/tree-ssa-loop-ivopts.c	(revision 246724)
+++ gcc/tree-ssa-loop-ivopts.c	(working copy)
@@ -7396,7 +7396,11 @@  rewrite_use_address (struct ivopts_data
     base_hint = var_at_stmt (data->current_loop, cand, use->stmt);
 
   iv = var_at_stmt (data->current_loop, cand, use->stmt);
-  ref = create_mem_ref (&bsi, TREE_TYPE (*use->op_p), &aff,
+  tree type = TREE_TYPE (*use->op_p);
+  unsigned int align = get_object_alignment (*use->op_p);
+  if (align != TYPE_ALIGN (type))
+    type = build_aligned_type (type, align);
+  ref = create_mem_ref (&bsi, type, &aff,
 			reference_alias_ptr_type (*use->op_p),
 			iv, base_hint, data->speed);
   copy_ref_info (ref, *use->op_p);
Index: gcc/testsuite/g++.dg/torture/pr80334.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr80334.C	(nonexistent)
+++ gcc/testsuite/g++.dg/torture/pr80334.C	(working copy)
@@ -0,0 +1,18 @@ 
+// { dg-do run }
+
+struct A { alignas(16) char c; };
+struct B { A unpacked; char d; } __attribute__((packed));
+
+char x;
+
+int
+main()
+{
+  alignas(16) B b[3];
+  for (int i = 0; i < 3; i++) b[i].unpacked.c = 'a' + i;
+  for (int i = 0; i < 3; i++)
+    {
+      auto a = new A(b[i].unpacked);
+      x = a->c;
+    }
+}