diff mbox

[committed] Fix ICE in gen_reg_rtx, at emit-rtl.c:864/865 compiling GNU MPFR

Message ID 87sj6j7mqn.fsf@talisman.default
State New
Headers show

Commit Message

Richard Sandiford Jan. 2, 2013, 1:12 p.m. UTC 
John David Anglin <dave@hiauly1.hia.nrc.ca> writes:
> The attached change fixes PR target/5379.  ICE occurs when reload tries
> to emit a move instruction containing a TLS symbol reference as the source
> operand.  This requires several scratch registers.  As a result, it isn't
> possible for a reload pattern to handle this case.  So, the best solution
> was to reject TLS symbol reference source operands after reload starts.
>
> Tested on hppa-unknown-linux-gnu and hppa2.0w-hp-hpux11.11 with not observed
> regressions.
>
> Committed to all active branches.
>
> Dave
>
> 2012-12-25  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>
>
> 	PR target/53789
> 	* config/pa/pa.md (movsi): Reject expansion of TLS symbol references
> 	after reload starts.
>
> Index: config/pa/pa.md
> ===================================================================
> --- config/pa/pa.md	(revision 194709)
> +++ config/pa/pa.md	(working copy)
> @@ -2051,6 +2110,12 @@
>    ""
>    "
>  {
> +  /* A TLS symbol reference is not a valid move source operand.
> +     pa_emit_move_sequence can only handle them prior to reload.
> +     There is also no way to reload a TLS symbol reference, so
> +     we must reject them after reload starts.  */
> +  if (PA_SYMBOL_REF_TLS_P (operands[1]) && !can_create_pseudo_p ())
> +    FAIL;
>    if (pa_emit_move_sequence (operands, SImode, 0))
>      DONE;
>  }")

Sorry to butt in, but I'm not sure about this.  Move patterns aren't
supposed to fail.  The nasty thing is that all FAIL actually does is
make the generator return null, and emitting a null insn is a no-op.
That means code like:

  /* from emit_move_insn_1 */
  code = optab_handler (mov_optab, mode);
  if (code != CODE_FOR_nothing)
    return emit_insn (GEN_FCN (code) (x, y));
  ...

  emit_move_insn{,_1} (x, y);

won't generate an ICE on FAIL, but it won't generate any insns either.
If someone's feeling brave, they might want to add an assert for nonnull.

In any case, reload needs to know up-front that the constant can't be
rematerialised, via LEGITIMATE_CONSTANT_P.  emit_move_insn_1 would be
too late.  It looks like PA already handles this for some TLS models:

  /* TLS_MODEL_GLOBAL_DYNAMIC and TLS_MODEL_LOCAL_DYNAMIC are not
     legitimate constants.  */
  if (PA_SYMBOL_REF_TLS_P (x))
   {
     enum tls_model model = SYMBOL_REF_TLS_MODEL (x);

     if (model == TLS_MODEL_GLOBAL_DYNAMIC || model == TLS_MODEL_LOCAL_DYNAMIC)
       return false;
   }

so maybe that should just be:

  if (PA_SYMBOL_REF_TLS_P (x))
    return false;

?  It probably ought to handle symbol + constant too though.

FWIW, I wrote the test below just to make sure that this worked for
mips64-linux-gnu.  OK to install?

...although I just tried it on x86_64-linux-gnu and it ICEs there.

Richard


gcc/testsuite/
	* gcc.dg/torture/tls/tls-reload-1.c: New test.

Comments

Richard Henderson Jan. 2, 2013, 6 p.m. UTC | #1 
On 01/02/2013 05:12 AM, Richard Sandiford wrote:
> 	* gcc.dg/torture/tls/tls-reload-1.c: New test.

Ok.


r~
Richard Sandiford Jan. 2, 2013, 7:44 p.m. UTC | #2 
Richard Henderson <rth@redhat.com> writes:
> On 01/02/2013 05:12 AM, Richard Sandiford wrote:
>> 	* gcc.dg/torture/tls/tls-reload-1.c: New test.
>
> Ok.

Thanks, committed.  And sorry for not volunteering a patch for the x86 ICE,
but I barely know the port...

Richard
diff mbox

Patch

Index: gcc/testsuite/gcc.dg/torture/tls/tls-reload-1.c
===================================================================
--- /dev/null	2012-12-29 10:15:03.199289441 +0000
+++ gcc/testsuite/gcc.dg/torture/tls/tls-reload-1.c	2013-01-02 12:56:22.278837979 +0000
@@ -0,0 +1,48 @@ 
+/* { dg-do run } */
+/* { dg-require-effective-target tls_runtime } */
+
+#define ARRAY(X) X##_array
+#define DECLARE(X) \
+  __thread int X; \
+  __thread int ARRAY(X)[4]; \
+  int *volatile *__attribute__((noinline)) \
+  check##X (int *volatile *y) \
+  { \
+    if (!y || *y++ != &X || *y++ != &ARRAY(X)[3]) \
+      return 0; \
+    return y; \
+  }
+#define COPY(X) *y++ = &X; *y++ = &ARRAY(X)[3];
+#define CHECK(X) y = check##X (y);
+#define A(M, X) M(X##0) M(X##1) M(X##2) M(X##3) M(X##4) M(X##5) M(X##6) M(X##7)
+#define B(M, X) A(M, X##0) A(M, X##1) A(M, X##2)
+#define C(M, X) B(M, X) B(M, X) B(M, X)
+
+#define NM 2
+#define NA (NM * 8)
+#define NB (NA * 3)
+#define NC (NB * 3)
+
+extern void abort (void);
+
+B(DECLARE, tls)
+
+void __attribute__ ((noinline))
+setup (int *volatile *y)
+{
+  C(COPY, tls)
+}
+
+int
+main (void)
+{
+  int *volatile array[NC];
+  int *volatile *y = array;
+  int i;
+
+  setup (array);
+  B(CHECK, tls);
+  if (!y)
+    abort ();
+  return 0;
+}