Re: [ARM] PR target/49030: ICE in get_arm_condition_code
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.
Re: [ARM] PR target/49030: ICE in get_arm_condition_code
> > 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
Re: [ARM] PR target/49030: ICE in get_arm_condition_code
Chung-Lin Tang 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
Re: [ARM] PR target/49030: ICE in get_arm_condition_code
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_L
[ARM] PR target/49030: ICE in get_arm_condition_code
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.c2011-09-02 15:46:44.013865635 +0100
+++ gcc/config/arm/arm.c2011-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;
