Patchwork Fix PR53703

login
register
mail settings
Submitter William J. Schmidt
Date June 18, 2012, 4:05 a.m.
Message ID <1339992345.18291.33.camel@gnopaine>
Download mbox | patch
Permalink /patch/165383/
State New
Headers show

Comments

William J. Schmidt - June 18, 2012, 4:05 a.m.
The test case exposes a bug that occurs only when a diamond control flow
pattern has the arguments of the joining phi in a different order from
the successor arcs of the entry block.  My logic for setting
bb_for_def[12] was just brain-dead.  This cleans that up and also
prevents wasting time examining phis of virtual ops, which I noticed
happening while debugging this.

Bootstrapped and regtested on powerpc64-unknown-linux-gnu with no new
failures.  Ok for trunk?

Thanks,
Bill


gcc:

2012-06-17  Bill Schmidt  <wschmidt@linux.ibm.com>

	PR tree-optimization/53703
	* tree-ssa-phiopt.c (hoist_adjacent_loads): Skip virtual phis;
	correctly set bb_for_def[12].

gcc/testsuite:

2012-06-17  Bill Schmidt  <wschmidt@linux.ibm.com>

	PR tree-optimization/53703
	* gcc.dg/torture/pr53703.c: New test.
Richard Guenther - June 18, 2012, 8:20 a.m.
On Sun, 17 Jun 2012, William J. Schmidt wrote:

> The test case exposes a bug that occurs only when a diamond control flow
> pattern has the arguments of the joining phi in a different order from
> the successor arcs of the entry block.  My logic for setting
> bb_for_def[12] was just brain-dead.  This cleans that up and also
> prevents wasting time examining phis of virtual ops, which I noticed
> happening while debugging this.
> 
> Bootstrapped and regtested on powerpc64-unknown-linux-gnu with no new
> failures.  Ok for trunk?

Ok.

Thanks,
Richard.

> Thanks,
> Bill
> 
> 
> gcc:
> 
> 2012-06-17  Bill Schmidt  <wschmidt@linux.ibm.com>
> 
> 	PR tree-optimization/53703
> 	* tree-ssa-phiopt.c (hoist_adjacent_loads): Skip virtual phis;
> 	correctly set bb_for_def[12].
> 
> gcc/testsuite:
> 
> 2012-06-17  Bill Schmidt  <wschmidt@linux.ibm.com>
> 
> 	PR tree-optimization/53703
> 	* gcc.dg/torture/pr53703.c: New test.
> 
> 
> Index: gcc/testsuite/gcc.dg/torture/pr53703.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/torture/pr53703.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/torture/pr53703.c	(revision 0)
> @@ -0,0 +1,149 @@
> +/* Reduced test case from PR53703.  Used to ICE.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-w" } */
> +
> +typedef long unsigned int size_t;
> +typedef unsigned short int sa_family_t;
> +struct sockaddr   {};
> +typedef unsigned char __u8;
> +typedef unsigned short __u16;
> +typedef unsigned int __u32;
> +struct nlmsghdr {
> +  __u32 nlmsg_len;
> +  __u16 nlmsg_type;
> +};
> +struct ifaddrmsg {
> +  __u8 ifa_family;
> +};
> +enum {
> +  IFA_ADDRESS,
> +  IFA_LOCAL,
> +};
> +enum {
> +  RTM_NEWLINK = 16,
> +  RTM_NEWADDR = 20,
> +};
> +struct rtattr {
> +  unsigned short rta_len;
> +  unsigned short rta_type;
> +};
> +struct ifaddrs {
> +  struct ifaddrs *ifa_next;
> +  unsigned short ifa_flags;
> +};
> +typedef unsigned short int uint16_t;
> +typedef unsigned int uint32_t;
> +struct nlmsg_list {
> +  struct nlmsg_list *nlm_next;
> +  int size;
> +};
> +struct rtmaddr_ifamap {
> +  void *address;
> +  void *local;
> +  int address_len;
> +  int local_len;
> +};
> +int usagi_getifaddrs (struct ifaddrs **ifap)
> +{
> +  struct nlmsg_list *nlmsg_list, *nlmsg_end, *nlm;
> +  size_t dlen, xlen, nlen;
> +  int build;
> +  for (build = 0; build <= 1; build++)
> +    {
> +      struct ifaddrs *ifl = ((void *)0), *ifa = ((void *)0);
> +      struct nlmsghdr *nlh, *nlh0;
> +      uint16_t *ifflist = ((void *)0);
> +      struct rtmaddr_ifamap ifamap;
> +      for (nlm = nlmsg_list; nlm; nlm = nlm->nlm_next)
> +	{
> +	  int nlmlen = nlm->size;
> +	  for (nlh = nlh0;
> +	       ((nlmlen) >= (int)sizeof(struct nlmsghdr)
> +		&& (nlh)->nlmsg_len >= sizeof(struct nlmsghdr)
> +		&& (nlh)->nlmsg_len <= (nlmlen));
> +	       nlh = ((nlmlen) -= ( (((nlh)->nlmsg_len)+4U -1) & ~(4U -1) ),
> +		      (struct nlmsghdr*)(((char*)(nlh))
> +					 + ( (((nlh)->nlmsg_len)+4U -1)
> +					     & ~(4U -1) ))))
> +	    {
> +	      struct ifinfomsg *ifim = ((void *)0);
> +	      struct ifaddrmsg *ifam = ((void *)0);
> +	      struct rtattr *rta;
> +	      sa_family_t nlm_family = 0;
> +	      uint32_t nlm_scope = 0, nlm_index = 0;
> +	      memset (&ifamap, 0, sizeof (ifamap));
> +	      switch (nlh->nlmsg_type)
> +		{
> +		case RTM_NEWLINK:
> +		  ifim = (struct ifinfomsg *)
> +		    ((void*)(((char*)nlh)
> +			     + ((0)+( ((((int)
> +					 ( ((sizeof(struct nlmsghdr))+4U -1)
> +					   & ~(4U -1) )))+4U -1)
> +				      & ~(4U -1) ))));
> +		case RTM_NEWADDR:
> +		  ifam = (struct ifaddrmsg *)
> +		    ((void*)(((char*)nlh)
> +			     + ((0)+( ((((int)
> +					 ( ((sizeof(struct nlmsghdr))+4U -1)
> +					   & ~(4U -1) )))+4U -1)
> +				      & ~(4U -1) ))));
> +		  nlm_family = ifam->ifa_family;
> +		  if (build)
> +		    ifa->ifa_flags = ifflist[nlm_index];
> +		  break;
> +		default:
> +		  continue;
> +		}
> +	      if (!build)
> +		{
> +		  void *rtadata = ((void*)(((char*)(rta))
> +					   + (( ((sizeof(struct rtattr))+4 -1)
> +						& ~(4 -1) ) + (0))));
> +		  size_t rtapayload = ((int)((rta)->rta_len)
> +				       - (( ((sizeof(struct rtattr))+4 -1)
> +					    & ~(4 -1) ) + (0)));
> +		  switch (nlh->nlmsg_type)
> +		    {
> +		    case RTM_NEWLINK:
> +		      break;
> +		    case RTM_NEWADDR:
> +		      if (nlm_family == 17)
> +			break;
> +		      switch (rta->rta_type)
> +			{
> +			case IFA_ADDRESS:
> +			  ifamap.address = rtadata;
> +			  ifamap.address_len = rtapayload;
> +			case IFA_LOCAL:
> +			  ifamap.local = rtadata;
> +			}
> +		    }
> +		}
> +	      if (nlh->nlmsg_type == RTM_NEWADDR && nlm_family != 17)
> +		{
> +		  if (!ifamap.local)
> +		    {
> +		      ifamap.local = ifamap.address;
> +		      ifamap.local_len = ifamap.address_len;
> +		    }
> +		  if (!ifamap.address)
> +		    {
> +		      ifamap.address = ifamap.local;
> +		    }
> +		  if (ifamap.address_len != ifamap.local_len
> +		      || (ifamap.address != ((void *)0)
> +			  && memcmp (ifamap.address, ifamap.local,
> +				     ifamap.address_len)))
> +		    {
> +		      if (!build)
> +			dlen += (((ifa_sa_len (nlm_family,
> +					       ifamap.address_len))+4U -1)
> +				 & ~(4U -1) );
> +		    }
> +		}
> +	    }
> +	}
> +    }
> +}
> Index: gcc/tree-ssa-phiopt.c
> ===================================================================
> --- gcc/tree-ssa-phiopt.c	(revision 188509)
> +++ gcc/tree-ssa-phiopt.c	(working copy)
> @@ -1853,7 +1853,9 @@ hoist_adjacent_loads (basic_block bb0, basic_block
>        if (TREE_CODE (arg1) != SSA_NAME
>  	  || TREE_CODE (arg2) != SSA_NAME
>  	  || SSA_NAME_IS_DEFAULT_DEF (arg1)
> -	  || SSA_NAME_IS_DEFAULT_DEF (arg2))
> +	  || SSA_NAME_IS_DEFAULT_DEF (arg2)
> +	  || !is_gimple_reg (arg1)
> +	  || !is_gimple_reg (arg2))
>  	continue;
>  
>        def1 = SSA_NAME_DEF_STMT (arg1);
> @@ -1914,17 +1916,11 @@ hoist_adjacent_loads (basic_block bb0, basic_block
>  	  defswap = def1;
>  	  def1 = def2;
>  	  def2 = defswap;
> -	  /* Don't swap bb1 and bb2 as we may have more than one
> -	     phi to process successfully.  */
> -	  bb_for_def1 = bb2;
> -	  bb_for_def2 = bb1;
>  	}
> -      else
> -	{
> -	  bb_for_def1 = bb1;
> -	  bb_for_def2 = bb2;
> -	}
>  
> +      bb_for_def1 = gimple_bb (def1);
> +      bb_for_def2 = gimple_bb (def2);
> +
>        /* Check for proper alignment of the first field.  */
>        tree_offset1 = bit_position (field1);
>        tree_offset2 = bit_position (field2);
> 
> 
>

Patch

Index: gcc/testsuite/gcc.dg/torture/pr53703.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr53703.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr53703.c	(revision 0)
@@ -0,0 +1,149 @@ 
+/* Reduced test case from PR53703.  Used to ICE.  */
+
+/* { dg-do compile } */
+/* { dg-options "-w" } */
+
+typedef long unsigned int size_t;
+typedef unsigned short int sa_family_t;
+struct sockaddr   {};
+typedef unsigned char __u8;
+typedef unsigned short __u16;
+typedef unsigned int __u32;
+struct nlmsghdr {
+  __u32 nlmsg_len;
+  __u16 nlmsg_type;
+};
+struct ifaddrmsg {
+  __u8 ifa_family;
+};
+enum {
+  IFA_ADDRESS,
+  IFA_LOCAL,
+};
+enum {
+  RTM_NEWLINK = 16,
+  RTM_NEWADDR = 20,
+};
+struct rtattr {
+  unsigned short rta_len;
+  unsigned short rta_type;
+};
+struct ifaddrs {
+  struct ifaddrs *ifa_next;
+  unsigned short ifa_flags;
+};
+typedef unsigned short int uint16_t;
+typedef unsigned int uint32_t;
+struct nlmsg_list {
+  struct nlmsg_list *nlm_next;
+  int size;
+};
+struct rtmaddr_ifamap {
+  void *address;
+  void *local;
+  int address_len;
+  int local_len;
+};
+int usagi_getifaddrs (struct ifaddrs **ifap)
+{
+  struct nlmsg_list *nlmsg_list, *nlmsg_end, *nlm;
+  size_t dlen, xlen, nlen;
+  int build;
+  for (build = 0; build <= 1; build++)
+    {
+      struct ifaddrs *ifl = ((void *)0), *ifa = ((void *)0);
+      struct nlmsghdr *nlh, *nlh0;
+      uint16_t *ifflist = ((void *)0);
+      struct rtmaddr_ifamap ifamap;
+      for (nlm = nlmsg_list; nlm; nlm = nlm->nlm_next)
+	{
+	  int nlmlen = nlm->size;
+	  for (nlh = nlh0;
+	       ((nlmlen) >= (int)sizeof(struct nlmsghdr)
+		&& (nlh)->nlmsg_len >= sizeof(struct nlmsghdr)
+		&& (nlh)->nlmsg_len <= (nlmlen));
+	       nlh = ((nlmlen) -= ( (((nlh)->nlmsg_len)+4U -1) & ~(4U -1) ),
+		      (struct nlmsghdr*)(((char*)(nlh))
+					 + ( (((nlh)->nlmsg_len)+4U -1)
+					     & ~(4U -1) ))))
+	    {
+	      struct ifinfomsg *ifim = ((void *)0);
+	      struct ifaddrmsg *ifam = ((void *)0);
+	      struct rtattr *rta;
+	      sa_family_t nlm_family = 0;
+	      uint32_t nlm_scope = 0, nlm_index = 0;
+	      memset (&ifamap, 0, sizeof (ifamap));
+	      switch (nlh->nlmsg_type)
+		{
+		case RTM_NEWLINK:
+		  ifim = (struct ifinfomsg *)
+		    ((void*)(((char*)nlh)
+			     + ((0)+( ((((int)
+					 ( ((sizeof(struct nlmsghdr))+4U -1)
+					   & ~(4U -1) )))+4U -1)
+				      & ~(4U -1) ))));
+		case RTM_NEWADDR:
+		  ifam = (struct ifaddrmsg *)
+		    ((void*)(((char*)nlh)
+			     + ((0)+( ((((int)
+					 ( ((sizeof(struct nlmsghdr))+4U -1)
+					   & ~(4U -1) )))+4U -1)
+				      & ~(4U -1) ))));
+		  nlm_family = ifam->ifa_family;
+		  if (build)
+		    ifa->ifa_flags = ifflist[nlm_index];
+		  break;
+		default:
+		  continue;
+		}
+	      if (!build)
+		{
+		  void *rtadata = ((void*)(((char*)(rta))
+					   + (( ((sizeof(struct rtattr))+4 -1)
+						& ~(4 -1) ) + (0))));
+		  size_t rtapayload = ((int)((rta)->rta_len)
+				       - (( ((sizeof(struct rtattr))+4 -1)
+					    & ~(4 -1) ) + (0)));
+		  switch (nlh->nlmsg_type)
+		    {
+		    case RTM_NEWLINK:
+		      break;
+		    case RTM_NEWADDR:
+		      if (nlm_family == 17)
+			break;
+		      switch (rta->rta_type)
+			{
+			case IFA_ADDRESS:
+			  ifamap.address = rtadata;
+			  ifamap.address_len = rtapayload;
+			case IFA_LOCAL:
+			  ifamap.local = rtadata;
+			}
+		    }
+		}
+	      if (nlh->nlmsg_type == RTM_NEWADDR && nlm_family != 17)
+		{
+		  if (!ifamap.local)
+		    {
+		      ifamap.local = ifamap.address;
+		      ifamap.local_len = ifamap.address_len;
+		    }
+		  if (!ifamap.address)
+		    {
+		      ifamap.address = ifamap.local;
+		    }
+		  if (ifamap.address_len != ifamap.local_len
+		      || (ifamap.address != ((void *)0)
+			  && memcmp (ifamap.address, ifamap.local,
+				     ifamap.address_len)))
+		    {
+		      if (!build)
+			dlen += (((ifa_sa_len (nlm_family,
+					       ifamap.address_len))+4U -1)
+				 & ~(4U -1) );
+		    }
+		}
+	    }
+	}
+    }
+}
Index: gcc/tree-ssa-phiopt.c
===================================================================
--- gcc/tree-ssa-phiopt.c	(revision 188509)
+++ gcc/tree-ssa-phiopt.c	(working copy)
@@ -1853,7 +1853,9 @@  hoist_adjacent_loads (basic_block bb0, basic_block
       if (TREE_CODE (arg1) != SSA_NAME
 	  || TREE_CODE (arg2) != SSA_NAME
 	  || SSA_NAME_IS_DEFAULT_DEF (arg1)
-	  || SSA_NAME_IS_DEFAULT_DEF (arg2))
+	  || SSA_NAME_IS_DEFAULT_DEF (arg2)
+	  || !is_gimple_reg (arg1)
+	  || !is_gimple_reg (arg2))
 	continue;
 
       def1 = SSA_NAME_DEF_STMT (arg1);
@@ -1914,17 +1916,11 @@  hoist_adjacent_loads (basic_block bb0, basic_block
 	  defswap = def1;
 	  def1 = def2;
 	  def2 = defswap;
-	  /* Don't swap bb1 and bb2 as we may have more than one
-	     phi to process successfully.  */
-	  bb_for_def1 = bb2;
-	  bb_for_def2 = bb1;
 	}
-      else
-	{
-	  bb_for_def1 = bb1;
-	  bb_for_def2 = bb2;
-	}
 
+      bb_for_def1 = gimple_bb (def1);
+      bb_for_def2 = gimple_bb (def2);
+
       /* Check for proper alignment of the first field.  */
       tree_offset1 = bit_position (field1);
       tree_offset2 = bit_position (field2);