There's a subtle bug in reorg.c's relax_delay_slots that my tester
tripped this weekend. Not sure what changed code generation wise as the
affected port built just fine last week. But it is what is is.
Assume before this code we've set TARGET_LABEL to the code_label
associated with DELAY_JUMP_INSN (which is what we want)...
/* If the first insn at TARGET_LABEL is redundant with a previous
insn, redirect the jump to the following insn and process again.
We use next_real_insn instead of next_active_insn so we
don't skip USE-markers, or we'll end up with incorrect
liveness info. */
[ ... ]
/* Similarly, if it is an unconditional jump with one insn in its
delay list and that insn is redundant, thread the jump. */
rtx_sequence *trial_seq =
trial ? dyn_cast <rtx_sequence *> (PATTERN (trial)) : NULL;
if (trial_seq
&& trial_seq->len () == 2
&& JUMP_P (trial_seq->insn (0))
&& simplejump_or_return_p (trial_seq->insn (0))
&& redundant_insn (trial_seq->insn (1), insn, vNULL))
{
target_label = JUMP_LABEL (trial_seq->insn (0));
if (ANY_RETURN_P (target_label))
target_label = find_end_label (target_label);
if (target_label
&& redirect_with_delay_slots_safe_p (delay_jump_insn,
target_label, insn))
{
update_block (trial_seq->insn (1), insn);
reorg_redirect_jump (delay_jump_insn, target_label);
next = insn;
continue;
}
}
/* See if we have a simple (conditional) jump that is useless. */
if (! INSN_ANNULLED_BRANCH_P (delay_jump_insn)
&& ! condjump_in_parallel_p (delay_jump_insn)
&& prev_active_insn (as_a<rtx_insn *> (target_label)) == insn
Now assume that we get into the TRUE arm of that first conditional which
sets a new value for TARGET_LABEL. Normally when this happens in
relax_delay_slots we're going to unconditionally continue. But as we
can see there's an inner conditional and if we don't get into its true
arm, then we'll pop out a nesting level and execute the second outer IF.
That second outer IF assumes that TARGET_LABEL still points to the code
label associated with DELAY_JUMP_INSN. Opps. In my particular case it
was NULL and caused an ICE. But I could probably construct a case where
it pointed to a real label and could result in incorrect code generation.
The fix is pretty simple. Just creating a new variable (to avoid
-Wshadow) inside that first outer IF is sufficient.
Tested by building the affected target (mipsisa64r2el-elf) through newlib.
Installed on the trunk.
jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 8cceb247a85..18b6ed59c73 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,8 @@
2017-05-15 Jeff Law <l...@redhat.com>
+ * reorg.c (relax_delay_slots): Create a new variable to hold
+ the temporary target rather than clobbering TARGET_LABEL.
+
* config/tilegx/tilegx.c (tilegx_expand_unaligned_load): Add
missing argument to extract_bit_field call.
* config/tilepro/tilepro.c (tilepro_expand_unaligned_load): Likewise.
diff --git a/gcc/reorg.c b/gcc/reorg.c
index 85ef7d6880c..1a6fd86e286 100644
--- a/gcc/reorg.c
+++ b/gcc/reorg.c
@@ -3351,16 +3351,16 @@ relax_delay_slots (rtx_insn *first)
&& simplejump_or_return_p (trial_seq->insn (0))
&& redundant_insn (trial_seq->insn (1), insn, vNULL))
{
- target_label = JUMP_LABEL (trial_seq->insn (0));
- if (ANY_RETURN_P (target_label))
- target_label = find_end_label (target_label);
+ rtx temp_label = JUMP_LABEL (trial_seq->insn (0));
+ if (ANY_RETURN_P (temp_label))
+ temp_label = find_end_label (temp_label);
- if (target_label
+ if (temp_label
&& redirect_with_delay_slots_safe_p (delay_jump_insn,
- target_label, insn))
+ temp_label, insn))
{
update_block (trial_seq->insn (1), insn);
- reorg_redirect_jump (delay_jump_insn, target_label);
+ reorg_redirect_jump (delay_jump_insn, temp_label);
next = insn;
continue;
}
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index f198fc6b42b..9fb8c8598b8 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2017-05-15 Jeff Law <l...@redhat.com>
+
+ * gcc.target/mips/reorgbug-1.c: New test.
+
2017-05-15 Pierre-Marie de Rodat <dero...@adacore.com>
* gnat.dg/specs/pack13.ads: New test.
diff --git a/gcc/testsuite/gcc.target/mips/reorgbug-1.c
b/gcc/testsuite/gcc.target/mips/reorgbug-1.c
new file mode 100644
index 00000000000..b820a2b5df1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/reorgbug-1.c
@@ -0,0 +1,39 @@
+/* { dg-options "-O2 -msoft-float -mips2" } */
+
+typedef long int __int32_t;
+typedef long unsigned int __uint32_t;
+typedef union
+{
+ double value;
+ struct
+ {
+ __uint32_t msw;
+ __uint32_t lsw;
+ }
+ parts;
+}
+ieee_double_shape_type;
+double
+__ieee754_fmod (double x, double y, int z, int xx)
+{
+ __int32_t n, hx, hy, hz, ix, iy, sx, i;
+ __uint32_t lx, ly, lz;
+ ieee_double_shape_type ew_u;
+ ew_u.value = (x);
+ (lx) = ew_u.parts.lsw;
+ ew_u.value = (y);
+ (hy) = ew_u.parts.msw;
+ (ly) = ew_u.parts.lsw;
+ if (hy == 0 || hx >= 0x7ff00000)
+ return (x * y);
+ if (z)
+ {
+ if ((hx < hy) || (lx < ly))
+ return x;
+ lz = lx - ly;
+ lx = lz + lz;
+ }
+ ieee_double_shape_type iw_u;
+ iw_u.parts.lsw = (lx);
+ return iw_u.value;
+}
