Patchwork Fix PR55882

login
register
mail settings
Submitter Richard Guenther
Date Jan. 15, 2013, 2:50 p.m.
Message ID <alpine.LNX.2.00.1301151548230.6889@zhemvz.fhfr.qr>
Download mbox | patch
Permalink /patch/212200/
State New
Headers show

Comments

Richard Guenther - Jan. 15, 2013, 2:50 p.m.
On Tue, 15 Jan 2013, Eric Botcazou wrote:

> > I have applied the minimal fix to trunk now.  We retain the clearly
> > incorrect fact that the !align_computed path only ever increases
> > alignment (it's only ever set previously to either BITS_PER_UNIT
> > or via the MEM_REF block).  I will momentarily test a followup
> > removing that block for trunk.
> 
> What do we do for the 4.7 branch?  The patch doesn't compile for it so I 
> presume that there are one or more changes to be backported first?

Not necessarily.  The following is a 4.7 variant of the patch
(on trunk get_object_alignment_1 got one extra output which
moved the align return value to by-reference).

Can you give it testing on the branch and a strict-align target?

Thanks,
Richard.

2013-01-15  Richard Biener  <rguenther@suse.de>

	PR middle-end/55882
	* emit-rtl.c (set_mem_attributes_minus_bitpos): Correctly
	account for bitpos when computing alignment.

	* gcc.dg/torture/pr55882.c: New testcase.

Index: gcc/testsuite/gcc.dg/torture/pr55882.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/pr55882.c	(revision 0)
--- gcc/testsuite/gcc.dg/torture/pr55882.c	(working copy)
***************
*** 0 ****
--- 1,94 ----
+ /* { dg-do run } */
+ 
+ typedef enum
+ {
+   PVT_A = 0,
+   PVT_B = 1,
+   PVT_CONFIG = 2,
+   PVT_RESERVED3 = 3,
+ } T_CR_SELECT;
+ 
+ typedef enum
+ {
+   STD_ULOGIC_0 = 0,
+   STD_ULOGIC_1 = 1,
+ } STD_ULOGIC;
+ 
+ typedef struct
+ {
+   unsigned char rtp : 3;
+   unsigned char rtn : 3;
+ } C;
+ 
+ typedef struct
+ {
+   unsigned char nd;
+   unsigned char pd;
+   unsigned char rtn;
+   unsigned char rtp;
+ } A;
+ 
+ typedef struct
+ {
+   unsigned short reserved : 14;
+   unsigned char Z_rx_enable : 2;
+   A pvt;
+ } B;
+ 
+ typedef struct
+ {
+   B cr_dsclk_q3;
+   B cr_data_q3;
+   B cr_addr_q3;
+   B cr_cmd_q3;
+   B cr_pres_q3;
+   C cr_vref_q3[6];
+   unsigned char pres_disable;
+   unsigned char pres_drive_high;
+   unsigned char c_enab_120;
+   STD_ULOGIC clk_tximp;
+   STD_ULOGIC dqs_tximp;
+   STD_ULOGIC cmd_tximp;
+   STD_ULOGIC data_tximp;
+   STD_ULOGIC dqs_rxterm;
+   STD_ULOGIC data_rxterm;
+   T_CR_SELECT cr_clk_sel;
+   unsigned char cr_clk : 5;
+   T_CR_SELECT cr_dsclk_odd_sel;
+   unsigned char cr_dsclk_odd : 5;
+   T_CR_SELECT cr_dsclk_even_sel;
+   unsigned char cr_dsclk_even : 5;
+   T_CR_SELECT cr_data_sel;
+   unsigned char cr_data : 5;
+   T_CR_SELECT cr_vref_sel;
+   unsigned char cr_vref : 5;
+   T_CR_SELECT cr_others_sel;
+   unsigned char cr_others : 5;
+ } CONFIG;
+ 
+ typedef struct
+ {
+   unsigned char enable_monitor;
+   unsigned short step_out_pointer : 12;
+   unsigned short hold_out_pointer : 12;
+   unsigned short enable_wr_dqs : 12;
+   unsigned short use_alt_rd_dqs : 12;
+   CONFIG io_buf;
+ } mystruct;
+ 
+ unsigned short __attribute__((noinline,noclone))
+ testfunction(unsigned i)
+ {
+   mystruct dmfe[8];
+   dmfe[0].use_alt_rd_dqs = 1;
+   dmfe[i].use_alt_rd_dqs = 0;
+   return dmfe[0].use_alt_rd_dqs;
+ }
+ 
+ extern void abort (void);
+ int main ()
+ {
+   if (testfunction(0) != 0) 
+     abort ();
+   return 0;
+ }
Eric Botcazou - Jan. 15, 2013, 11:34 p.m.
> Not necessarily.  The following is a 4.7 variant of the patch
> (on trunk get_object_alignment_1 got one extra output which
> moved the align return value to by-reference).

OK, I obviously didn't try very hard here...

> Can you give it testing on the branch and a strict-align target?
> 
> Thanks,
> Richard.
> 
> 2013-01-15  Richard Biener  <rguenther@suse.de>
> 
> 	PR middle-end/55882
> 	* emit-rtl.c (set_mem_attributes_minus_bitpos): Correctly
> 	account for bitpos when computing alignment.
> 
> 	* gcc.dg/torture/pr55882.c: New testcase.

Bootstrapped/regtested on 4.7 branch for SPARC/Solaris, all clear.
Richard Guenther - Jan. 16, 2013, 9:26 a.m.
On Wed, 16 Jan 2013, Eric Botcazou wrote:

> > Not necessarily.  The following is a 4.7 variant of the patch
> > (on trunk get_object_alignment_1 got one extra output which
> > moved the align return value to by-reference).
> 
> OK, I obviously didn't try very hard here...
> 
> > Can you give it testing on the branch and a strict-align target?
> > 
> > Thanks,
> > Richard.
> > 
> > 2013-01-15  Richard Biener  <rguenther@suse.de>
> > 
> > 	PR middle-end/55882
> > 	* emit-rtl.c (set_mem_attributes_minus_bitpos): Correctly
> > 	account for bitpos when computing alignment.
> > 
> > 	* gcc.dg/torture/pr55882.c: New testcase.
> 
> Bootstrapped/regtested on 4.7 branch for SPARC/Solaris, all clear.

Thanks, committed on the branch as well.

The situation with the MEM_REF block on trunk isn't all that bad,
it can't result in bogus alignment as far as I can see.  Thus I'll leave
further cleanups to when stage1 opens again.

Richard.

Patch

Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	(revision 195201)
+++ gcc/emit-rtl.c	(working copy)
@@ -1839,7 +1839,12 @@  set_mem_attributes_minus_bitpos (rtx ref
 
       if (!align_computed)
 	{
-	  unsigned int obj_align = get_object_alignment (t);
+	  unsigned int obj_align;
+	  unsigned HOST_WIDE_INT obj_bitpos;
+	  obj_align = get_object_alignment_1 (t, &obj_bitpos);
+	  obj_bitpos = (obj_bitpos - bitpos) & (obj_align - 1);
+	  if (obj_bitpos != 0)
+	    obj_align = (obj_bitpos & -obj_bitpos);
 	  attrs.align = MAX (attrs.align, obj_align);
 	}
     }