public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [ARM] PR target/49030: ICE in get_arm_condition_code
@ 2011-09-02 15:01 Richard Sandiford
  2011-09-02 15:58 ` Chung-Lin Tang
  2011-09-07 10:55 ` Richard Earnshaw
  0 siblings, 2 replies; 5+ messages in thread
From: Richard Sandiford @ 2011-09-02 15:01 UTC (permalink / raw)
  To: gcc-patches

CC_NCV rightly only allows GE(U) and LT(U).  GT(U) and LE(U) have to
implemented by reversing the condition.  This is handled correctly when
the condition is first expanded, but nothing stops later optimisers from
producing invalid forms.

This patch makes arm_comparison_operator check that the condition
is acceptable.  Tested on arm-linux-gnueabi.  OK to install?

Richard


gcc/
	* config/arm/arm-protos.h (maybe_get_arm_condition_code): Declare.
	* config/arm/arm.c (maybe_get_arm_condition_code): New function,
	reusing the old code from get_arm_condition_code.  Return ARM_NV
	for invalid comparison codes.
	(get_arm_condition_code): Redefine in terms of
	maybe_get_arm_condition_code.
	* config/arm/predicates.md (arm_comparison_operator): Use
	maybe_get_arm_condition_code.

gcc/testsuite/
	* gcc.dg/torture/pr49030.c: New test.

Index: gcc/config/arm/arm-protos.h
===================================================================
--- gcc/config/arm/arm-protos.h	2011-09-02 15:46:44.013865635 +0100
+++ gcc/config/arm/arm-protos.h	2011-09-02 15:56:35.749477269 +0100
@@ -184,6 +184,7 @@ extern int is_called_in_ARM_mode (tree);
 #endif
 extern int thumb_shiftable_const (unsigned HOST_WIDE_INT);
 #ifdef RTX_CODE
+extern enum arm_cond_code maybe_get_arm_condition_code (rtx);
 extern void thumb1_final_prescan_insn (rtx);
 extern void thumb2_final_prescan_insn (rtx);
 extern const char *thumb_load_double_from_address (rtx *);
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	2011-09-02 15:46:44.013865635 +0100
+++ gcc/config/arm/arm.c	2011-09-02 15:56:35.756477252 +0100
@@ -17595,10 +17595,10 @@ arm_elf_asm_destructor (rtx symbol, int 
    decremented/zeroed by arm_asm_output_opcode as the insns are output.  */
 
 /* Returns the index of the ARM condition code string in
-   `arm_condition_codes'.  COMPARISON should be an rtx like
-   `(eq (...) (...))'.  */
-static enum arm_cond_code
-get_arm_condition_code (rtx comparison)
+   `arm_condition_codes', or ARM_NV if the comparison is invalid.
+   COMPARISON should be an rtx like `(eq (...) (...))'.  */
+enum arm_cond_code
+maybe_get_arm_condition_code (rtx comparison)
 {
   enum machine_mode mode = GET_MODE (XEXP (comparison, 0));
   enum arm_cond_code code;
@@ -17622,11 +17622,11 @@ get_arm_condition_code (rtx comparison)
     case CC_DLTUmode: code = ARM_CC;
 
     dominance:
-      gcc_assert (comp_code == EQ || comp_code == NE);
-
       if (comp_code == EQ)
 	return ARM_INVERSE_CONDITION_CODE (code);
-      return code;
+      if (comp_code == NE)
+	return code;
+      return ARM_NV;
 
     case CC_NOOVmode:
       switch (comp_code)
@@ -17635,7 +17635,7 @@ get_arm_condition_code (rtx comparison)
 	case EQ: return ARM_EQ;
 	case GE: return ARM_PL;
 	case LT: return ARM_MI;
-	default: gcc_unreachable ();
+	default: return ARM_NV;
 	}
 
     case CC_Zmode:
@@ -17643,7 +17643,7 @@ get_arm_condition_code (rtx comparison)
 	{
 	case NE: return ARM_NE;
 	case EQ: return ARM_EQ;
-	default: gcc_unreachable ();
+	default: return ARM_NV;
 	}
 
     case CC_Nmode:
@@ -17651,7 +17651,7 @@ get_arm_condition_code (rtx comparison)
 	{
 	case NE: return ARM_MI;
 	case EQ: return ARM_PL;
-	default: gcc_unreachable ();
+	default: return ARM_NV;
 	}
 
     case CCFPEmode:
@@ -17676,7 +17676,7 @@ get_arm_condition_code (rtx comparison)
 	  /* UNEQ and LTGT do not have a representation.  */
 	case UNEQ: /* Fall through.  */
 	case LTGT: /* Fall through.  */
-	default: gcc_unreachable ();
+	default: return ARM_NV;
 	}
 
     case CC_SWPmode:
@@ -17692,7 +17692,7 @@ get_arm_condition_code (rtx comparison)
 	case GTU: return ARM_CC;
 	case LEU: return ARM_CS;
 	case LTU: return ARM_HI;
-	default: gcc_unreachable ();
+	default: return ARM_NV;
 	}
 
     case CC_Cmode:
@@ -17700,7 +17700,7 @@ get_arm_condition_code (rtx comparison)
 	{
 	case LTU: return ARM_CS;
 	case GEU: return ARM_CC;
-	default: gcc_unreachable ();
+	default: return ARM_NV;
 	}
 
     case CC_CZmode:
@@ -17712,7 +17712,7 @@ get_arm_condition_code (rtx comparison)
 	case GTU: return ARM_HI;
 	case LEU: return ARM_LS;
 	case LTU: return ARM_CC;
-	default: gcc_unreachable ();
+	default: return ARM_NV;
 	}
 
     case CC_NCVmode:
@@ -17722,7 +17722,7 @@ get_arm_condition_code (rtx comparison)
 	case LT: return ARM_LT;
 	case GEU: return ARM_CS;
 	case LTU: return ARM_CC;
-	default: gcc_unreachable ();
+	default: return ARM_NV;
 	}
 
     case CCmode:
@@ -17738,13 +17738,22 @@ get_arm_condition_code (rtx comparison)
 	case GTU: return ARM_HI;
 	case LEU: return ARM_LS;
 	case LTU: return ARM_CC;
-	default: gcc_unreachable ();
+	default: return ARM_NV;
 	}
 
     default: gcc_unreachable ();
     }
 }
 
+/* Like maybe_get_arm_condition_code, but never return ARM_NV.  */
+static enum arm_cond_code
+get_arm_condition_code (rtx comparison)
+{
+  enum arm_cond_code code = maybe_get_arm_condition_code (comparison);
+  gcc_assert (code != ARM_NV);
+  return code;
+}
+
 /* Tell arm_asm_output_opcode to output IT blocks for conditionally executed
    instructions.  */
 void
Index: gcc/config/arm/predicates.md
===================================================================
--- gcc/config/arm/predicates.md	2011-09-02 15:46:44.013865635 +0100
+++ gcc/config/arm/predicates.md	2011-09-02 15:56:35.764477233 +0100
@@ -249,10 +249,9 @@ (define_special_predicate "equality_oper
 ;; True for integer comparisons and, if FP is active, for comparisons
 ;; other than LTGT or UNEQ.
 (define_special_predicate "arm_comparison_operator"
-  (ior (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu")
-       (and (match_test "TARGET_32BIT && TARGET_HARD_FLOAT
-			 && (TARGET_FPA || TARGET_VFP)")
-            (match_code "unordered,ordered,unlt,unle,unge,ungt"))))
+  (and (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,
+		    unordered,ordered,unlt,unle,unge,ungt")
+       (match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
 
 (define_special_predicate "lt_ge_comparison_operator"
   (match_code "lt,ge"))
Index: gcc/testsuite/gcc.dg/torture/pr49030.c
===================================================================
--- /dev/null	2011-08-09 12:56:23.674000002 +0100
+++ gcc/testsuite/gcc.dg/torture/pr49030.c	2011-09-02 15:56:42.803460791 +0100
@@ -0,0 +1,19 @@
+void
+sample_move_d32u24_sS (char *dst, float *src, unsigned long nsamples,
+		       unsigned long dst_skip)
+{
+  long long y;
+  while (nsamples--)
+    {
+      y = (long long) (*src * 8388608.0f) << 8;
+      if (y > 2147483647) {
+	*(int *) dst = 2147483647;
+      } else if (y < -2147483647 - 1) {
+	*(int *) dst = -2147483647 - 1;
+      } else {
+	*(int *) dst = (int) y;
+      }
+      dst += dst_skip;
+      src++;
+    }
+}

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [ARM] PR target/49030: ICE in get_arm_condition_code
  2011-09-02 15:01 [ARM] PR target/49030: ICE in get_arm_condition_code Richard Sandiford
@ 2011-09-02 15:58 ` Chung-Lin Tang
  2011-09-06 11:29   ` Richard Sandiford
  2011-09-07 10:55 ` Richard Earnshaw
  1 sibling, 1 reply; 5+ messages in thread
From: Chung-Lin Tang @ 2011-09-02 15:58 UTC (permalink / raw)
  To: gcc-patches, richard.sandiford

Hi Richard, this looks very similar to this patch, originally for LP:689887:
http://gcc.gnu.org/ml/gcc-patches/2011-01/msg00794.html
Apart from your additional handling in the dominance modes cases.

I remember that last patch was held down because Thumb-2 native
bootstrap failed. Did you try that test?

Thanks,
Chung-Lin

On 2011/9/2 11:01 PM, Richard Sandiford wrote:
> CC_NCV rightly only allows GE(U) and LT(U).  GT(U) and LE(U) have to
> implemented by reversing the condition.  This is handled correctly when
> the condition is first expanded, but nothing stops later optimisers from
> producing invalid forms.
> 
> This patch makes arm_comparison_operator check that the condition
> is acceptable.  Tested on arm-linux-gnueabi.  OK to install?
> 
> Richard
> 
> 
> gcc/
> 	* config/arm/arm-protos.h (maybe_get_arm_condition_code): Declare.
> 	* config/arm/arm.c (maybe_get_arm_condition_code): New function,
> 	reusing the old code from get_arm_condition_code.  Return ARM_NV
> 	for invalid comparison codes.
> 	(get_arm_condition_code): Redefine in terms of
> 	maybe_get_arm_condition_code.
> 	* config/arm/predicates.md (arm_comparison_operator): Use
> 	maybe_get_arm_condition_code.
> 
> gcc/testsuite/
> 	* gcc.dg/torture/pr49030.c: New test.
> 
> Index: gcc/config/arm/arm-protos.h
> ===================================================================
> --- gcc/config/arm/arm-protos.h	2011-09-02 15:46:44.013865635 +0100
> +++ gcc/config/arm/arm-protos.h	2011-09-02 15:56:35.749477269 +0100
> @@ -184,6 +184,7 @@ extern int is_called_in_ARM_mode (tree);
>  #endif
>  extern int thumb_shiftable_const (unsigned HOST_WIDE_INT);
>  #ifdef RTX_CODE
> +extern enum arm_cond_code maybe_get_arm_condition_code (rtx);
>  extern void thumb1_final_prescan_insn (rtx);
>  extern void thumb2_final_prescan_insn (rtx);
>  extern const char *thumb_load_double_from_address (rtx *);
> Index: gcc/config/arm/arm.c
> ===================================================================
> --- gcc/config/arm/arm.c	2011-09-02 15:46:44.013865635 +0100
> +++ gcc/config/arm/arm.c	2011-09-02 15:56:35.756477252 +0100
> @@ -17595,10 +17595,10 @@ arm_elf_asm_destructor (rtx symbol, int 
>     decremented/zeroed by arm_asm_output_opcode as the insns are output.  */
>  
>  /* Returns the index of the ARM condition code string in
> -   `arm_condition_codes'.  COMPARISON should be an rtx like
> -   `(eq (...) (...))'.  */
> -static enum arm_cond_code
> -get_arm_condition_code (rtx comparison)
> +   `arm_condition_codes', or ARM_NV if the comparison is invalid.
> +   COMPARISON should be an rtx like `(eq (...) (...))'.  */
> +enum arm_cond_code
> +maybe_get_arm_condition_code (rtx comparison)
>  {
>    enum machine_mode mode = GET_MODE (XEXP (comparison, 0));
>    enum arm_cond_code code;
> @@ -17622,11 +17622,11 @@ get_arm_condition_code (rtx comparison)
>      case CC_DLTUmode: code = ARM_CC;
>  
>      dominance:
> -      gcc_assert (comp_code == EQ || comp_code == NE);
> -
>        if (comp_code == EQ)
>  	return ARM_INVERSE_CONDITION_CODE (code);
> -      return code;
> +      if (comp_code == NE)
> +	return code;
> +      return ARM_NV;
>  
>      case CC_NOOVmode:
>        switch (comp_code)
> @@ -17635,7 +17635,7 @@ get_arm_condition_code (rtx comparison)
>  	case EQ: return ARM_EQ;
>  	case GE: return ARM_PL;
>  	case LT: return ARM_MI;
> -	default: gcc_unreachable ();
> +	default: return ARM_NV;
>  	}
>  
>      case CC_Zmode:
> @@ -17643,7 +17643,7 @@ get_arm_condition_code (rtx comparison)
>  	{
>  	case NE: return ARM_NE;
>  	case EQ: return ARM_EQ;
> -	default: gcc_unreachable ();
> +	default: return ARM_NV;
>  	}
>  
>      case CC_Nmode:
> @@ -17651,7 +17651,7 @@ get_arm_condition_code (rtx comparison)
>  	{
>  	case NE: return ARM_MI;
>  	case EQ: return ARM_PL;
> -	default: gcc_unreachable ();
> +	default: return ARM_NV;
>  	}
>  
>      case CCFPEmode:
> @@ -17676,7 +17676,7 @@ get_arm_condition_code (rtx comparison)
>  	  /* UNEQ and LTGT do not have a representation.  */
>  	case UNEQ: /* Fall through.  */
>  	case LTGT: /* Fall through.  */
> -	default: gcc_unreachable ();
> +	default: return ARM_NV;
>  	}
>  
>      case CC_SWPmode:
> @@ -17692,7 +17692,7 @@ get_arm_condition_code (rtx comparison)
>  	case GTU: return ARM_CC;
>  	case LEU: return ARM_CS;
>  	case LTU: return ARM_HI;
> -	default: gcc_unreachable ();
> +	default: return ARM_NV;
>  	}
>  
>      case CC_Cmode:
> @@ -17700,7 +17700,7 @@ get_arm_condition_code (rtx comparison)
>  	{
>  	case LTU: return ARM_CS;
>  	case GEU: return ARM_CC;
> -	default: gcc_unreachable ();
> +	default: return ARM_NV;
>  	}
>  
>      case CC_CZmode:
> @@ -17712,7 +17712,7 @@ get_arm_condition_code (rtx comparison)
>  	case GTU: return ARM_HI;
>  	case LEU: return ARM_LS;
>  	case LTU: return ARM_CC;
> -	default: gcc_unreachable ();
> +	default: return ARM_NV;
>  	}
>  
>      case CC_NCVmode:
> @@ -17722,7 +17722,7 @@ get_arm_condition_code (rtx comparison)
>  	case LT: return ARM_LT;
>  	case GEU: return ARM_CS;
>  	case LTU: return ARM_CC;
> -	default: gcc_unreachable ();
> +	default: return ARM_NV;
>  	}
>  
>      case CCmode:
> @@ -17738,13 +17738,22 @@ get_arm_condition_code (rtx comparison)
>  	case GTU: return ARM_HI;
>  	case LEU: return ARM_LS;
>  	case LTU: return ARM_CC;
> -	default: gcc_unreachable ();
> +	default: return ARM_NV;
>  	}
>  
>      default: gcc_unreachable ();
>      }
>  }
>  
> +/* Like maybe_get_arm_condition_code, but never return ARM_NV.  */
> +static enum arm_cond_code
> +get_arm_condition_code (rtx comparison)
> +{
> +  enum arm_cond_code code = maybe_get_arm_condition_code (comparison);
> +  gcc_assert (code != ARM_NV);
> +  return code;
> +}
> +
>  /* Tell arm_asm_output_opcode to output IT blocks for conditionally executed
>     instructions.  */
>  void
> Index: gcc/config/arm/predicates.md
> ===================================================================
> --- gcc/config/arm/predicates.md	2011-09-02 15:46:44.013865635 +0100
> +++ gcc/config/arm/predicates.md	2011-09-02 15:56:35.764477233 +0100
> @@ -249,10 +249,9 @@ (define_special_predicate "equality_oper
>  ;; True for integer comparisons and, if FP is active, for comparisons
>  ;; other than LTGT or UNEQ.
>  (define_special_predicate "arm_comparison_operator"
> -  (ior (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu")
> -       (and (match_test "TARGET_32BIT && TARGET_HARD_FLOAT
> -			 && (TARGET_FPA || TARGET_VFP)")
> -            (match_code "unordered,ordered,unlt,unle,unge,ungt"))))
> +  (and (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,
> +		    unordered,ordered,unlt,unle,unge,ungt")
> +       (match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
>  
>  (define_special_predicate "lt_ge_comparison_operator"
>    (match_code "lt,ge"))
> Index: gcc/testsuite/gcc.dg/torture/pr49030.c
> ===================================================================
> --- /dev/null	2011-08-09 12:56:23.674000002 +0100
> +++ gcc/testsuite/gcc.dg/torture/pr49030.c	2011-09-02 15:56:42.803460791 +0100
> @@ -0,0 +1,19 @@
> +void
> +sample_move_d32u24_sS (char *dst, float *src, unsigned long nsamples,
> +		       unsigned long dst_skip)
> +{
> +  long long y;
> +  while (nsamples--)
> +    {
> +      y = (long long) (*src * 8388608.0f) << 8;
> +      if (y > 2147483647) {
> +	*(int *) dst = 2147483647;
> +      } else if (y < -2147483647 - 1) {
> +	*(int *) dst = -2147483647 - 1;
> +      } else {
> +	*(int *) dst = (int) y;
> +      }
> +      dst += dst_skip;
> +      src++;
> +    }
> +}

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [ARM] PR target/49030: ICE in get_arm_condition_code
  2011-09-02 15:58 ` Chung-Lin Tang
@ 2011-09-06 11:29   ` Richard Sandiford
  2011-09-07  8:28     ` Ramana Radhakrishnan
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Sandiford @ 2011-09-06 11:29 UTC (permalink / raw)
  To: Chung-Lin Tang; +Cc: gcc-patches

Chung-Lin Tang <cltang@codesourcery.com> writes:
> Hi Richard, this looks very similar to this patch, originally for LP:689887:
> http://gcc.gnu.org/ml/gcc-patches/2011-01/msg00794.html
> Apart from your additional handling in the dominance modes cases.

Indeed.  Sorry about that.  It must look odd that I've posted such
a similar patch to a bug that I'd also assigned to myself.  The reason
is that this problem was reported more recently as two other Launchpad
bugs, and I only found the old one when submitting the patch (I linked the
new bugs to the bugzilla PR, and Launchpad told me about the old one also
being linked to that PR).  I didn't even notice there was a patch attached.

My patch has now survived a Thumb-2 bootstrap, and it sounds like Ramana
has also successfully bootstrapped your original patch.

Richard

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [ARM] PR target/49030: ICE in get_arm_condition_code
  2011-09-06 11:29   ` Richard Sandiford
@ 2011-09-07  8:28     ` Ramana Radhakrishnan
  0 siblings, 0 replies; 5+ messages in thread
From: Ramana Radhakrishnan @ 2011-09-07  8:28 UTC (permalink / raw)
  To: Chung-Lin Tang, gcc-patches, richard.sandiford; +Cc: Richard Earnshaw

>
> My patch has now survived a Thumb-2 bootstrap, and it sounds like Ramana
> has also successfully bootstrapped your original patch.

I'd rather take the version that handles the cases for the dominance
modes as well. RichardE, do you think you could have a look at this
one ?

cheers
Ramana

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [ARM] PR target/49030: ICE in get_arm_condition_code
  2011-09-02 15:01 [ARM] PR target/49030: ICE in get_arm_condition_code Richard Sandiford
  2011-09-02 15:58 ` Chung-Lin Tang
@ 2011-09-07 10:55 ` Richard Earnshaw
  1 sibling, 0 replies; 5+ messages in thread
From: Richard Earnshaw @ 2011-09-07 10:55 UTC (permalink / raw)
  To: gcc-patches, richard.sandiford

On 02/09/11 16:01, Richard Sandiford wrote:
> CC_NCV rightly only allows GE(U) and LT(U).  GT(U) and LE(U) have to
> implemented by reversing the condition.  This is handled correctly when
> the condition is first expanded, but nothing stops later optimisers from
> producing invalid forms.
> 
> This patch makes arm_comparison_operator check that the condition
> is acceptable.  Tested on arm-linux-gnueabi.  OK to install?
> 
> Richard
> 
> 
> gcc/
> 	* config/arm/arm-protos.h (maybe_get_arm_condition_code): Declare.
> 	* config/arm/arm.c (maybe_get_arm_condition_code): New function,
> 	reusing the old code from get_arm_condition_code.  Return ARM_NV
> 	for invalid comparison codes.
> 	(get_arm_condition_code): Redefine in terms of
> 	maybe_get_arm_condition_code.
> 	* config/arm/predicates.md (arm_comparison_operator): Use
> 	maybe_get_arm_condition_code.
> 
> gcc/testsuite/
> 	* gcc.dg/torture/pr49030.c: New test.

OK.

R.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-09-07 10:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-02 15:01 [ARM] PR target/49030: ICE in get_arm_condition_code Richard Sandiford
2011-09-02 15:58 ` Chung-Lin Tang
2011-09-06 11:29   ` Richard Sandiford
2011-09-07  8:28     ` Ramana Radhakrishnan
2011-09-07 10:55 ` Richard Earnshaw

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).