[committed] Fix pr92085-2.c regressions on msp430-elf

Message ID b0d732c7f0bdedd128dc89036523608ab5a3287d.camel@redhat.com
State New
Headers show
Series
  • [committed] Fix pr92085-2.c regressions on msp430-elf
Related show

Commit Message

Kees Cook via Gcc-patches June 1, 2020, 9:19 p.m.
msp430-elf has had regressions for a while.  There was other instability at the
time the regression started, so I waited to see if it'd correct itself, but it
didn't and I finally took a looksie.

We're processing this in lower-subreg:

(insn 30 64 42 6 (set (subreg:SI (reg/v:HI 33 [ oz ]) 0)
        (concatn:SI [
                (reg:HI 45 [ _5 ])
                (reg:HI 46 [ _5+2 ])
            ])) "j.c":22:19 14 {movsi_x}
     (nil))


Note the paradoxical subreg destination.  There's nothing inherently wrong with
that.    But if lower-subreg wants to decompose it it'll use
simplify_gen_subreg_concatn which has this gem:

 /* If we see an insn like (set (reg:DI) (subreg:DI (reg:SI) 0)) then
     resolve_simple_move will ask for the high part of the paradoxical
     subreg, which does not have a value.  Just return a zero.  */
  if (ret == NULL_RTX
      && paradoxical_subreg_p (op))
    return CONST0_RTX (outermode);

That's fine and good for the source of a set, but it's not good for the
destination of a set.  For the destination we might as well just not emit
anything.  The bits we're setting are don't cares and leaving them uninitialized
should be fine.

And that's exactly what this patch does.  WHen simplify_gen_subreg_concatn
returns CONST0_RTX for a destination operand, we assume that we need not actually
assign anything to the destination and leave it as-is.

This fixed the regression on the msp430-elf port and has bootstrapped and
successfully regression tested on x86_64-linux-gnu.  It's built on a few other
targets as well, but I haven't tried to enumerate them -- I just knew there
aren't new failures since dropping this patch into my tester :-)


Installing on the trunk,

Jeff
commit c7969df1c5d3785c0b409f97e7682a6f0d2637ec
Author: Jeff Law <law@torsion.usersys.redhat.com>
Date:   Mon Jun 1 17:14:50 2020 -0400

    Fix 92085-2.c ICE due to having (const_int 0) as the destination of a set.
    
    gcc/
            * lower-subreg.c (resolve_simple_move): If simplify_gen_subreg_concatn
            returns (const_int 0) for the destination, then emit nothing.

Patch

diff --git a/gcc/lower-subreg.c b/gcc/lower-subreg.c
index a11e535b5bf..abe7180c686 100644
--- a/gcc/lower-subreg.c
+++ b/gcc/lower-subreg.c
@@ -1087,12 +1087,21 @@  resolve_simple_move (rtx set, rtx_insn *insn)
 	emit_clobber (dest);
 
       for (i = 0; i < words; ++i)
-	emit_move_insn (simplify_gen_subreg_concatn (word_mode, dest,
-						     dest_mode,
-						     i * UNITS_PER_WORD),
-			simplify_gen_subreg_concatn (word_mode, src,
-						     orig_mode,
-						     i * UNITS_PER_WORD));
+	{
+	  rtx t = simplify_gen_subreg_concatn (word_mode, dest,
+					       dest_mode,
+					       i * UNITS_PER_WORD);
+	  /* simplify_gen_subreg_concatn can return (const_int 0) for
+	     some sub-objects of paradoxical subregs.  As a source operand,
+	     that's fine.  As a destination it must be avoided.  Those are
+	     supposed to be don't care bits, so we can just drop that store
+	     on the floor.  */
+	  if (t != CONST0_RTX (word_mode))
+	    emit_move_insn (t,
+			    simplify_gen_subreg_concatn (word_mode, src,
+							 orig_mode,
+							 i * UNITS_PER_WORD));
+	}
     }
 
   if (real_dest != NULL_RTX)