[PATCH] Fix debug ICE on aarch64 due to bad rtl simplification (PR debug/70628)

[PATCH] Fix debug ICE on aarch64 due to bad rtl simplification (PR debug/70628)

[Jakub Jelinek]

Hi!

Most of simplify-rtx.c attempts hard not to generate any extra instructions,
just simplify the RTL to something equivalent and let the user try to
recognize it, or whatever else it wishes.
This is especially important post-reload, when we can't create new pseudos,
or e.g. when processing (simplifying) DEBUG_INSNs, where we certainly don't
want any code around it (not to mention where exactly would it be emitted
anyway?).  E.g. it uses the gen_lowpart_no_emit rtl hook instead of
gen_lowpart, etc.
As the following testcase shows, there are two exceptions, for
POINTER_EXTEND* targets under some very limited conditions it actually
calls convert_memory_address, which sometimes does what simplify-rtx.c
normally does, just return equivalent RTL expression, but in another case
can create a new pseudo (no no after reload, or for debug insns any time)
and emit some move insns.

This patch arranges for a new argument to
convert_memory_address_addr_space_1 and calls it with that new argument set
to true, to make sure it never emits instructions or creates pseudos.

Compared to the previous version of the patch, I've added ATTRIBUTE_UNUSED
to the new argument, so that bootstrap doesn't fail on targets that don't
define POINTERS_EXTEND*.

Bootstrapped/regtested on x86_64-linux and i686-linux, bootstrapped on
aarch64-linux (regtest pending).
I don't have access to aarch64 ilp32 setup though, Andrew or anybody else,
could you please test it there?  Also CCing H.J. just in case it affects
x86 -mx32.

Ok for trunk?

2016-04-12  Jakub Jelinek  <jakub@redhat.com>

	PR debug/70628
	* rtl.h (convert_memory_address_addr_space_1): New prototype.
	* explow.c (convert_memory_address_addr_space_1): No longer static,
	add NO_EMIT argument and don't call convert_modes if true, pass
	it down recursively, remove break after return.
	(convert_memory_address_addr_space): Adjust caller.
	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_addr_space_1 instead of convert_memory_address,
	if it returns NULL, don't simplify.

	* gcc.dg/torture/pr70628.c: New test.

--- gcc/rtl.h.jj	2016-04-04 12:28:42.000000000 +0200
+++ gcc/rtl.h	2016-04-12 12:08:00.327498386 +0200
@@ -2747,6 +2747,8 @@ extern unsigned int subreg_highpart_offs
 					    machine_mode);
 extern int byte_lowpart_offset (machine_mode, machine_mode);
 extern rtx make_safe_from (rtx, rtx);
+extern rtx convert_memory_address_addr_space_1 (machine_mode, rtx,
+						addr_space_t, bool, bool);
 extern rtx convert_memory_address_addr_space (machine_mode, rtx,
 					      addr_space_t);
 #define convert_memory_address(to_mode,x) \
--- gcc/explow.c.jj	2016-03-13 21:39:24.000000000 +0100
+++ gcc/explow.c	2016-04-12 13:27:26.945851321 +0200
@@ -259,12 +259,14 @@ break_out_memory_refs (rtx x)
    which way).  We take advantage of the fact that pointers are not allowed to
    overflow by commuting arithmetic operations over conversions so that address
    arithmetic insns can be used. IN_CONST is true if this conversion is inside
-   a CONST.  */
+   a CONST. NO_EMIT is true if no insns should be emitted, and instead
+   it should return NULL if it can't be simplified without emitting insns.  */
 
-static rtx
+rtx
 convert_memory_address_addr_space_1 (machine_mode to_mode ATTRIBUTE_UNUSED,
 				     rtx x, addr_space_t as ATTRIBUTE_UNUSED,
-				     bool in_const ATTRIBUTE_UNUSED)
+				     bool in_const ATTRIBUTE_UNUSED,
+				     bool no_emit ATTRIBUTE_UNUSED)
 {
 #ifndef POINTERS_EXTEND_UNSIGNED
   gcc_assert (GET_MODE (x) == to_mode || GET_MODE (x) == VOIDmode);
@@ -310,19 +312,16 @@ convert_memory_address_addr_space_1 (mac
       temp = gen_rtx_LABEL_REF (to_mode, LABEL_REF_LABEL (x));
       LABEL_REF_NONLOCAL_P (temp) = LABEL_REF_NONLOCAL_P (x);
       return temp;
-      break;
 
     case SYMBOL_REF:
       temp = shallow_copy_rtx (x);
       PUT_MODE (temp, to_mode);
       return temp;
-      break;
 
     case CONST:
       return gen_rtx_CONST (to_mode,
 			    convert_memory_address_addr_space_1
-			      (to_mode, XEXP (x, 0), as, true));
-      break;
+			      (to_mode, XEXP (x, 0), as, true, no_emit));
 
     case PLUS:
     case MULT:
@@ -338,11 +337,12 @@ convert_memory_address_addr_space_1 (mac
 	      && CONST_INT_P (XEXP (x, 1))
 	      && ((in_const && POINTERS_EXTEND_UNSIGNED != 0)
 		  || XEXP (x, 1) == convert_memory_address_addr_space_1
-				     (to_mode, XEXP (x, 1), as, in_const)
+				     (to_mode, XEXP (x, 1), as, in_const,
+				      no_emit)
                   || POINTERS_EXTEND_UNSIGNED < 0)))
 	return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
 			       convert_memory_address_addr_space_1
-				 (to_mode, XEXP (x, 0), as, in_const),
+				 (to_mode, XEXP (x, 0), as, in_const, no_emit),
 			       XEXP (x, 1));
       break;
 
@@ -350,6 +350,9 @@ convert_memory_address_addr_space_1 (mac
       break;
     }
 
+  if (no_emit)
+    return NULL_RTX;
+
   return convert_modes (to_mode, from_mode,
 			x, POINTERS_EXTEND_UNSIGNED);
 #endif /* defined(POINTERS_EXTEND_UNSIGNED) */
@@ -364,7 +367,7 @@ convert_memory_address_addr_space_1 (mac
 rtx
 convert_memory_address_addr_space (machine_mode to_mode, rtx x, addr_space_t as)
 {
-  return convert_memory_address_addr_space_1 (to_mode, x, as, false);
+  return convert_memory_address_addr_space_1 (to_mode, x, as, false, false);
 }
 \f
 
--- gcc/simplify-rtx.c.jj	2016-04-05 19:01:34.000000000 +0200
+++ gcc/simplify-rtx.c	2016-04-12 12:25:32.895790315 +0200
@@ -1482,7 +1482,14 @@ simplify_unary_operation_1 (enum rtx_cod
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode))
 	  && !targetm.have_ptr_extend ())
-	return convert_memory_address (Pmode, op);
+	{
+	  temp
+	    = convert_memory_address_addr_space_1 (Pmode, op,
+						   ADDR_SPACE_GENERIC, false,
+						   true);
+	  if (temp)
+	    return temp;
+	}
 #endif
       break;
 
@@ -1604,7 +1611,14 @@ simplify_unary_operation_1 (enum rtx_cod
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode))
 	  && !targetm.have_ptr_extend ())
-	return convert_memory_address (Pmode, op);
+	{
+	  temp
+	    = convert_memory_address_addr_space_1 (Pmode, op,
+						   ADDR_SPACE_GENERIC, false,
+						   true);
+	  if (temp)
+	    return temp;
+	}
 #endif
       break;
 
--- gcc/testsuite/gcc.dg/torture/pr70628.c.jj	2016-04-12 13:26:35.852566678 +0200
+++ gcc/testsuite/gcc.dg/torture/pr70628.c	2016-04-12 13:26:12.000000000 +0200
@@ -0,0 +1,46 @@
+/* PR debug/70628 */
+/* { dg-do compile } */
+/* { dg-options "-g -w" } */
+
+struct S { char s[64]; int *t; } *a;
+char b[64];
+int *foo (void);
+struct S *bar (int *);
+int baz (void);
+
+void
+test (const char *p, long q)
+{
+  int *c;
+  c = foo ();
+  while (a = bar (c))
+    {
+      if (__builtin_strstr (p, "ABCD")
+	  || __builtin_strstr (p, "EFGHI")
+	  || __builtin_strstr (p, "JKL")
+	  || __builtin_strstr (p, "MNOPQR")
+	  || __builtin_strstr (p, "STUV")
+	  || __builtin_strstr (p, "WXYZabcd")
+	  || __builtin_strstr (p, "efghij")
+	  || __builtin_strstr (p, "klmno")
+	  || __builtin_strstr (p, "pqrstuvw")
+	  || __builtin_strstr (b, "MNOPQR") != "EFGHI"
+	  || __builtin_strstr (b, "JKL"))
+	if (__builtin_strstr (a->s, "xyz12"))
+	  continue;
+      __builtin_printf ("%p\n", a->t);
+    }
+  bar (c);
+  while (a)
+    if (__builtin_strstr (p, "ABCD")
+	|| __builtin_strstr (p, "EFGHI")
+	|| __builtin_strstr (p, "JKL")
+	|| __builtin_strstr (p, "MNOPQR")
+	|| __builtin_strstr (p, "STUV")
+	|| __builtin_strstr (p, "WXYZabcd")
+	|| __builtin_strstr (p, "efghij")
+	|| __builtin_strstr (p, "klmno")
+	|| __builtin_strstr (p, "pqrstuvw")
+	|| __builtin_strstr ((const char *) q, "MNOPQR"))
+      baz ();
+}

	Jakub

Re: [PATCH] Fix debug ICE on aarch64 due to bad rtl simplification (PR debug/70628)

[H.J. Lu]

On Tue, Apr 12, 2016 at 8:10 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> Most of simplify-rtx.c attempts hard not to generate any extra instructions,
> just simplify the RTL to something equivalent and let the user try to
> recognize it, or whatever else it wishes.
> This is especially important post-reload, when we can't create new pseudos,
> or e.g. when processing (simplifying) DEBUG_INSNs, where we certainly don't
> want any code around it (not to mention where exactly would it be emitted
> anyway?).  E.g. it uses the gen_lowpart_no_emit rtl hook instead of
> gen_lowpart, etc.
> As the following testcase shows, there are two exceptions, for
> POINTER_EXTEND* targets under some very limited conditions it actually
> calls convert_memory_address, which sometimes does what simplify-rtx.c
> normally does, just return equivalent RTL expression, but in another case
> can create a new pseudo (no no after reload, or for debug insns any time)
> and emit some move insns.
>
> This patch arranges for a new argument to
> convert_memory_address_addr_space_1 and calls it with that new argument set
> to true, to make sure it never emits instructions or creates pseudos.
>
> Compared to the previous version of the patch, I've added ATTRIBUTE_UNUSED
> to the new argument, so that bootstrap doesn't fail on targets that don't
> define POINTERS_EXTEND*.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, bootstrapped on
> aarch64-linux (regtest pending).
> I don't have access to aarch64 ilp32 setup though, Andrew or anybody else,
> could you please test it there?  Also CCing H.J. just in case it affects
> x86 -mx32.
>

I bootstrapped and tested it on x32.  There is no regression.


-- 
H.J.

Re: [PATCH] Fix debug ICE on aarch64 due to bad rtl simplification (PR debug/70628)

[Bernd Schmidt]

On 04/12/2016 05:10 PM, Jakub Jelinek wrote:
> This patch arranges for a new argument to
> convert_memory_address_addr_space_1 and calls it with that new argument set
> to true, to make sure it never emits instructions or creates pseudos.

I think the approach looks sensible, but I don't know if you need the 
extra argument. It looks like convert_memory_address_addr_space_1 
currently only has one user, can't you move the convert_modes call up 
into that function so that convert_memory_address_addr_space_1 never 
emits anything?


Bernd

Re: [PATCH] Fix debug ICE on aarch64 due to bad rtl simplification (PR debug/70628)

[Jakub Jelinek]

On Tue, Apr 12, 2016 at 10:02:18PM +0200, Bernd Schmidt wrote:
> On 04/12/2016 05:10 PM, Jakub Jelinek wrote:
> >This patch arranges for a new argument to
> >convert_memory_address_addr_space_1 and calls it with that new argument set
> >to true, to make sure it never emits instructions or creates pseudos.
> 
> I think the approach looks sensible, but I don't know if you need the extra
> argument. It looks like convert_memory_address_addr_space_1 currently only
> has one user, can't you move the convert_modes call up into that function so
> that convert_memory_address_addr_space_1 never emits anything?

convert_memory_address_addr_space_1 is recursive function, so it has more
than one caller (convert_memory_address_addr_space and itself).

Which actually means that the patch is wrong, for no_emit on the recursive
calls it should actually check the result of the recursive call and
return NULL immediately if the recursive call returned NULL.

So, would you be ok with the patch if I change it that way (to be tested
tomorrow)?

	Jakub

Re: [PATCH] Fix debug ICE on aarch64 due to bad rtl simplification (PR debug/70628)

[Bernd Schmidt]

On 04/12/2016 10:06 PM, Jakub Jelinek wrote:
> On Tue, Apr 12, 2016 at 10:02:18PM +0200, Bernd Schmidt wrote:
>> On 04/12/2016 05:10 PM, Jakub Jelinek wrote:
>>> This patch arranges for a new argument to
>>> convert_memory_address_addr_space_1 and calls it with that new argument set
>>> to true, to make sure it never emits instructions or creates pseudos.
>>
>> I think the approach looks sensible, but I don't know if you need the extra
>> argument. It looks like convert_memory_address_addr_space_1 currently only
>> has one user, can't you move the convert_modes call up into that function so
>> that convert_memory_address_addr_space_1 never emits anything?
>
> convert_memory_address_addr_space_1 is recursive function, so it has more
> than one caller (convert_memory_address_addr_space and itself).

D'oh. Sorry.

> Which actually means that the patch is wrong, for no_emit on the recursive
> calls it should actually check the result of the recursive call and
> return NULL immediately if the recursive call returned NULL.

Hmm. I wonder whether this couldn't be restructured a bit to strip CONST 
internally and putting it back at the end. However, now is not the time 
for that.

> So, would you be ok with the patch if I change it that way (to be tested
> tomorrow)?

Yeah, I think so.


Bernd

Re: [PATCH] Fix debug ICE on aarch64 due to bad rtl simplification (PR debug/70628)

[Jakub Jelinek]

On Tue, Apr 12, 2016 at 10:14:17PM +0200, Bernd Schmidt wrote:
> >So, would you be ok with the patch if I change it that way (to be tested
> >tomorrow)?
> 
> Yeah, I think so.

So here is what I'll throw at testing:

2016-04-12  Jakub Jelinek  <jakub@redhat.com>

	PR debug/70628
	* rtl.h (convert_memory_address_addr_space_1): New prototype.
	* explow.c (convert_memory_address_addr_space_1): No longer static,
	add NO_EMIT argument and don't call convert_modes if true, pass
	it down recursively, remove break after return.
	(convert_memory_address_addr_space): Adjust caller.
	* simplify-rtx.c (simplify_unary_operation_1): Call
	convert_memory_address_addr_space_1 instead of convert_memory_address,
	if it returns NULL, don't simplify.

	* gcc.dg/torture/pr70628.c: New test.

--- gcc/rtl.h.jj	2016-04-12 19:12:34.681054428 +0200
+++ gcc/rtl.h	2016-04-12 22:20:14.879906956 +0200
@@ -2747,6 +2747,8 @@ extern unsigned int subreg_highpart_offs
 					    machine_mode);
 extern int byte_lowpart_offset (machine_mode, machine_mode);
 extern rtx make_safe_from (rtx, rtx);
+extern rtx convert_memory_address_addr_space_1 (machine_mode, rtx,
+						addr_space_t, bool, bool);
 extern rtx convert_memory_address_addr_space (machine_mode, rtx,
 					      addr_space_t);
 #define convert_memory_address(to_mode,x) \
--- gcc/explow.c.jj	2016-04-12 19:12:34.711054027 +0200
+++ gcc/explow.c	2016-04-12 22:26:17.633953657 +0200
@@ -259,12 +259,14 @@ break_out_memory_refs (rtx x)
    which way).  We take advantage of the fact that pointers are not allowed to
    overflow by commuting arithmetic operations over conversions so that address
    arithmetic insns can be used. IN_CONST is true if this conversion is inside
-   a CONST.  */
+   a CONST. NO_EMIT is true if no insns should be emitted, and instead
+   it should return NULL if it can't be simplified without emitting insns.  */
 
-static rtx
+rtx
 convert_memory_address_addr_space_1 (machine_mode to_mode ATTRIBUTE_UNUSED,
 				     rtx x, addr_space_t as ATTRIBUTE_UNUSED,
-				     bool in_const ATTRIBUTE_UNUSED)
+				     bool in_const ATTRIBUTE_UNUSED,
+				     bool no_emit ATTRIBUTE_UNUSED)
 {
 #ifndef POINTERS_EXTEND_UNSIGNED
   gcc_assert (GET_MODE (x) == to_mode || GET_MODE (x) == VOIDmode);
@@ -310,19 +312,16 @@ convert_memory_address_addr_space_1 (mac
       temp = gen_rtx_LABEL_REF (to_mode, LABEL_REF_LABEL (x));
       LABEL_REF_NONLOCAL_P (temp) = LABEL_REF_NONLOCAL_P (x);
       return temp;
-      break;
 
     case SYMBOL_REF:
       temp = shallow_copy_rtx (x);
       PUT_MODE (temp, to_mode);
       return temp;
-      break;
 
     case CONST:
-      return gen_rtx_CONST (to_mode,
-			    convert_memory_address_addr_space_1
-			      (to_mode, XEXP (x, 0), as, true));
-      break;
+      temp = convert_memory_address_addr_space_1 (to_mode, XEXP (x, 0), as,
+						  true, no_emit);
+      return temp ? gen_rtx_CONST (to_mode, temp) : temp;
 
     case PLUS:
     case MULT:
@@ -338,18 +337,25 @@ convert_memory_address_addr_space_1 (mac
 	      && CONST_INT_P (XEXP (x, 1))
 	      && ((in_const && POINTERS_EXTEND_UNSIGNED != 0)
 		  || XEXP (x, 1) == convert_memory_address_addr_space_1
-				     (to_mode, XEXP (x, 1), as, in_const)
+				     (to_mode, XEXP (x, 1), as, in_const,
+				      no_emit)
                   || POINTERS_EXTEND_UNSIGNED < 0)))
-	return gen_rtx_fmt_ee (GET_CODE (x), to_mode,
-			       convert_memory_address_addr_space_1
-				 (to_mode, XEXP (x, 0), as, in_const),
-			       XEXP (x, 1));
+	{
+	  temp = convert_memory_address_addr_space_1 (to_mode, XEXP (x, 0),
+						      as, in_const, no_emit);
+	  return temp ? gen_rtx_fmt_ee (GET_CODE (x), to_mode,
+					temp, XEXP (x, 1))
+		      : temp;
+	}
       break;
 
     default:
       break;
     }
 
+  if (no_emit)
+    return NULL_RTX;
+
   return convert_modes (to_mode, from_mode,
 			x, POINTERS_EXTEND_UNSIGNED);
 #endif /* defined(POINTERS_EXTEND_UNSIGNED) */
@@ -364,7 +370,7 @@ convert_memory_address_addr_space_1 (mac
 rtx
 convert_memory_address_addr_space (machine_mode to_mode, rtx x, addr_space_t as)
 {
-  return convert_memory_address_addr_space_1 (to_mode, x, as, false);
+  return convert_memory_address_addr_space_1 (to_mode, x, as, false, false);
 }
 \f
 
--- gcc/simplify-rtx.c.jj	2016-04-12 19:12:34.755053438 +0200
+++ gcc/simplify-rtx.c	2016-04-12 22:20:14.881906929 +0200
@@ -1482,7 +1482,14 @@ simplify_unary_operation_1 (enum rtx_cod
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode))
 	  && !targetm.have_ptr_extend ())
-	return convert_memory_address (Pmode, op);
+	{
+	  temp
+	    = convert_memory_address_addr_space_1 (Pmode, op,
+						   ADDR_SPACE_GENERIC, false,
+						   true);
+	  if (temp)
+	    return temp;
+	}
 #endif
       break;
 
@@ -1604,7 +1611,14 @@ simplify_unary_operation_1 (enum rtx_cod
 		  && REG_POINTER (SUBREG_REG (op))
 		  && GET_MODE (SUBREG_REG (op)) == Pmode))
 	  && !targetm.have_ptr_extend ())
-	return convert_memory_address (Pmode, op);
+	{
+	  temp
+	    = convert_memory_address_addr_space_1 (Pmode, op,
+						   ADDR_SPACE_GENERIC, false,
+						   true);
+	  if (temp)
+	    return temp;
+	}
 #endif
       break;
 
--- gcc/testsuite/gcc.dg/torture/pr70628.c.jj	2016-04-12 22:20:14.882906915 +0200
+++ gcc/testsuite/gcc.dg/torture/pr70628.c	2016-04-12 22:20:14.882906915 +0200
@@ -0,0 +1,46 @@
+/* PR debug/70628 */
+/* { dg-do compile } */
+/* { dg-options "-g -w" } */
+
+struct S { char s[64]; int *t; } *a;
+char b[64];
+int *foo (void);
+struct S *bar (int *);
+int baz (void);
+
+void
+test (const char *p, long q)
+{
+  int *c;
+  c = foo ();
+  while (a = bar (c))
+    {
+      if (__builtin_strstr (p, "ABCD")
+	  || __builtin_strstr (p, "EFGHI")
+	  || __builtin_strstr (p, "JKL")
+	  || __builtin_strstr (p, "MNOPQR")
+	  || __builtin_strstr (p, "STUV")
+	  || __builtin_strstr (p, "WXYZabcd")
+	  || __builtin_strstr (p, "efghij")
+	  || __builtin_strstr (p, "klmno")
+	  || __builtin_strstr (p, "pqrstuvw")
+	  || __builtin_strstr (b, "MNOPQR") != "EFGHI"
+	  || __builtin_strstr (b, "JKL"))
+	if (__builtin_strstr (a->s, "xyz12"))
+	  continue;
+      __builtin_printf ("%p\n", a->t);
+    }
+  bar (c);
+  while (a)
+    if (__builtin_strstr (p, "ABCD")
+	|| __builtin_strstr (p, "EFGHI")
+	|| __builtin_strstr (p, "JKL")
+	|| __builtin_strstr (p, "MNOPQR")
+	|| __builtin_strstr (p, "STUV")
+	|| __builtin_strstr (p, "WXYZabcd")
+	|| __builtin_strstr (p, "efghij")
+	|| __builtin_strstr (p, "klmno")
+	|| __builtin_strstr (p, "pqrstuvw")
+	|| __builtin_strstr ((const char *) q, "MNOPQR"))
+      baz ();
+}


	Jakub

Re: [PATCH] Fix debug ICE on aarch64 due to bad rtl simplification (PR debug/70628)

[Andreas Schwab]

Jakub Jelinek <jakub@redhat.com> writes:

> So here is what I'll throw at testing:
>
> 2016-04-12  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR debug/70628
> 	* rtl.h (convert_memory_address_addr_space_1): New prototype.
> 	* explow.c (convert_memory_address_addr_space_1): No longer static,
> 	add NO_EMIT argument and don't call convert_modes if true, pass
> 	it down recursively, remove break after return.
> 	(convert_memory_address_addr_space): Adjust caller.
> 	* simplify-rtx.c (simplify_unary_operation_1): Call
> 	convert_memory_address_addr_space_1 instead of convert_memory_address,
> 	if it returns NULL, don't simplify.

I have successfully bootstrapped it on ILP32 and verified that it fixes
the ICE.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Fix debug ICE on aarch64 due to bad rtl simplification (PR debug/70628)

[Bernd Schmidt]

On 04/12/2016 10:34 PM, Jakub Jelinek wrote:
> +	  return temp ? gen_rtx_fmt_ee (GET_CODE (x), to_mode,
> +					temp, XEXP (x, 1))
> +		      : temp;

Wrap multi-line expressions in parens. No need for a full retest, just 
make sure the file still builds.


Bernd