diff mbox

[PATCH/AARCH64] Fix PR 61345: rtx_cost ICEing on simple code

Message ID CA+=Sn1kGHKNOH1JCrgg-0p2tWS4g6C_oMTrsSbD9MJmffQ96+A@mail.gmail.com
State New
Headers show

Commit Message

Andrew Pinski May 28, 2014, 10:58 p.m. UTC 
Hi,
  The problem here is aarch64_rtx_costs for IF_THEN_ELSE does not
handle the case where the first operand is a non comparison.  This
happens when the combine is combing a few RTLs and calling
set_src_cost to check the costs of the newly created rtl.

At the same I noticed the code for rtx_costs was getting more complex
so I factored out the code for IF_THEN_ELSE

OK?  Built and tested on aarch64-elf with no regressions.

Thanks,
Andrew Pinski

ChangeLog:
* config/aarch64/aarch64.c (aarch64_if_then_else_costs): New function.
(aarch64_rtx_costs): Use aarch64_if_then_else_costs.

testsuite/ChangeLog:
* gcc.c-torture/compile/20140528-1.c: New testcase.
commit d2a89a0b21f13e676a863eeb3ac1f9ad927e65ac
Author: Andrew Pinski <apinski@cavium.com>
Date:   Wed May 28 15:44:05 2014 -0700

    Fix kernel.

Comments

Marcus Shawcroft June 2, 2014, 10:47 a.m. UTC | #1 
On 28 May 2014 23:58, Andrew Pinski <andrew.pinski@caviumnetworks.com> wrote:
> Hi,
>   The problem here is aarch64_rtx_costs for IF_THEN_ELSE does not
> handle the case where the first operand is a non comparison.  This
> happens when the combine is combing a few RTLs and calling
> set_src_cost to check the costs of the newly created rtl.
>
> At the same I noticed the code for rtx_costs was getting more complex
> so I factored out the code for IF_THEN_ELSE
>
> OK?  Built and tested on aarch64-elf with no regressions.
>
> Thanks,
> Andrew Pinski
>
> ChangeLog:
> * config/aarch64/aarch64.c (aarch64_if_then_else_costs): New function.
> (aarch64_rtx_costs): Use aarch64_if_then_else_costs.
>
> testsuite/ChangeLog:
> * gcc.c-torture/compile/20140528-1.c: New testcase.

Hi,  Can you split this into two patches please, the first a straight
refactor, no functional change, the second with the fix and testcase ?

+  enum rtx_code cmpcode;
+  if (COMPARISON_P (op0))

Blank line between declarations and statement please.

+  return false;
+
+}

Drop the superflous blank line after the return please.

Thanks
/Marcus
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index c2f6c4f..32c5fcb 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -4848,6 +4848,80 @@  aarch64_rtx_arith_op_extract_p (rtx x, enum machine_mode mode)
   return false;
 }
 
+/* Calculate the cost of calculating (if_then_else (OP0) (OP1) (OP2)),
+   storing it in *COST.  Result is true if the total cost of the operation
+   has now been calculated.  */
+static bool
+aarch64_if_then_else_costs (rtx op0, rtx op1, rtx op2, int *cost, bool speed)
+{
+  rtx inner;
+  rtx comparator;
+  enum rtx_code cmpcode;
+  if (COMPARISON_P (op0))
+    {
+      inner = XEXP (op0, 0);
+      comparator = XEXP (op0, 1);
+      cmpcode = GET_CODE (op0);
+    }
+  else
+    {
+      inner = op0;
+      comparator = const0_rtx;
+      cmpcode = NE;
+    }
+
+  if (GET_CODE (op1) == PC || GET_CODE (op2) == PC)
+    {
+      /* Conditional branch.  */
+      if (GET_MODE_CLASS (GET_MODE (inner)) == MODE_CC)
+	return true;
+      else
+	{
+	  if (cmpcode == NE || cmpcode == EQ)
+	    {
+	      if (comparator == const0_rtx)
+		{
+		  /* TBZ/TBNZ/CBZ/CBNZ.  */
+		  if (GET_CODE (inner) == ZERO_EXTRACT)
+		    /* TBZ/TBNZ.  */
+		    *cost += rtx_cost (XEXP (inner, 0), ZERO_EXTRACT,
+			 	       0, speed);
+		else
+		  /* CBZ/CBNZ.  */
+		  *cost += rtx_cost (inner, cmpcode, 0, speed);
+
+	        return true;
+	      }
+	    }
+	  else if (cmpcode == LT || cmpcode == GE)
+	    {
+	      /* TBZ/TBNZ.  */
+	      if (comparator == const0_rtx)
+		return true;
+	    }
+	}
+    }
+  else if (GET_MODE_CLASS (GET_MODE (inner)) == MODE_CC)
+    {
+      /* It's a conditional operation based on the status flags,
+	 so it must be some flavor of CSEL.  */
+
+      /* CSNEG, CSINV, and CSINC are handled for free as part of CSEL.  */
+      if (GET_CODE (op1) == NEG
+          || GET_CODE (op1) == NOT
+          || (GET_CODE (op1) == PLUS && XEXP (op1, 1) == const1_rtx))
+	op1 = XEXP (op1, 0);
+
+      *cost += rtx_cost (op1, IF_THEN_ELSE, 1, speed);
+      *cost += rtx_cost (op2, IF_THEN_ELSE, 2, speed);
+      return true;
+    }
+
+  /* We don't know what this is, cost all operands.  */
+  return false;
+
+}
+
 /* Calculate the cost of calculating X, storing it in *COST.  Result
    is true if the total cost of the operation has now been calculated.  */
 static bool
@@ -5582,67 +5656,9 @@  cost_plus:
       return false;  /* All arguments need to be in registers.  */
 
     case IF_THEN_ELSE:
-      op2 = XEXP (x, 2);
-      op0 = XEXP (x, 0);
-      op1 = XEXP (x, 1);
-
-      if (GET_CODE (op1) == PC || GET_CODE (op2) == PC)
-        {
-          /* Conditional branch.  */
-          if (GET_MODE_CLASS (GET_MODE (XEXP (op0, 0))) == MODE_CC)
-	    return true;
-	  else
-	    {
-	      if (GET_CODE (op0) == NE
-		  || GET_CODE (op0) == EQ)
-		{
-		  rtx inner = XEXP (op0, 0);
-		  rtx comparator = XEXP (op0, 1);
-
-		  if (comparator == const0_rtx)
-		    {
-		      /* TBZ/TBNZ/CBZ/CBNZ.  */
-		      if (GET_CODE (inner) == ZERO_EXTRACT)
-			/* TBZ/TBNZ.  */
-			*cost += rtx_cost (XEXP (inner, 0), ZERO_EXTRACT,
-					   0, speed);
-		      else
-			/* CBZ/CBNZ.  */
-			*cost += rtx_cost (inner, GET_CODE (op0), 0, speed);
-
-		      return true;
-		    }
-		}
-	      else if (GET_CODE (op0) == LT
-		       || GET_CODE (op0) == GE)
-		{
-		  rtx comparator = XEXP (op0, 1);
-
-		  /* TBZ/TBNZ.  */
-		  if (comparator == const0_rtx)
-		    return true;
-		}
-	    }
-        }
-      else if (GET_MODE_CLASS (GET_MODE (XEXP (op0, 0))) == MODE_CC)
-        {
-          /* It's a conditional operation based on the status flags,
-             so it must be some flavor of CSEL.  */
-
-          /* CSNEG, CSINV, and CSINC are handled for free as part of CSEL.  */
-          if (GET_CODE (op1) == NEG
-              || GET_CODE (op1) == NOT
-              || (GET_CODE (op1) == PLUS && XEXP (op1, 1) == const1_rtx))
-            op1 = XEXP (op1, 0);
-
-          *cost += rtx_cost (op1, IF_THEN_ELSE, 1, speed);
-          *cost += rtx_cost (op2, IF_THEN_ELSE, 2, speed);
-          return true;
-        }
-
-      /* We don't know what this is, cost all operands.  */
-      return false;
-
+      return aarch64_if_then_else_costs (XEXP (x, 0), XEXP (x, 1),
+					 XEXP (x, 2), cost, speed);
+      
     case EQ:
     case NE:
     case GT:
diff --git a/gcc/testsuite/gcc.c-torture/compile/20140528-1.c b/gcc/testsuite/gcc.c-torture/compile/20140528-1.c
new file mode 100644
index 0000000..d227802
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/20140528-1.c
@@ -0,0 +1,9 @@ 
+unsigned f(unsigned flags, unsigned capabilities)
+{
+  unsigned gfp_mask;
+  unsigned gfp_notmask = 0;
+  gfp_mask = flags & ((1 << 25) - 1);
+  if (!(capabilities & 0x00000001))
+    gfp_mask |= 0x1000000u;
+  return (gfp_mask & ~gfp_notmask);
+}