Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)

Submitted by Steven Bosscher on Nov. 27, 2012, 11:53 p.m.

Details

Message ID CABu31nPOwDmsvsD8Ge42n0G2eFxHhm9WWcAyx8ku8sChDnaXHQ@mail.gmail.com
State New
Headers show

Commit Message

Steven Bosscher Nov. 27, 2012, 11:53 p.m.
On Wed, Nov 28, 2012 at 12:47 AM, Eric Botcazou wrote:
>> Count me in, too.  So let's avoid it:
>>
>>         * gcse.c (struct reg_use): Remove unused struct.
>>         (gcse_emit_move_after): Do not create REG_EQUAL notes that reference
>> the SET_DEST of the instruction the note would be attached to.
>
> OK (with PR rtl-optimization/55006) if it passes bootstrap & regtest, thanks.

Thanks for the quick review.


>> And perhaps a bit in emit-rtl.c for good measure? I'll see if/where
>> this causes breakage...
>
> I think this would need to be wrapped in a #ifdef ENABLE_CHECKING/#endif pair.

Well, let me first try to make it pass some set of tests. This breaks
the compiler in too many places to name right now. Here's the first of
what's probably going to be a whole series of patches if I keep
hitting this internal_error as often as I am now in my set of cc1-i
files...

I know: ChangeLog, testing and all that. That's not the point yet.
This is just a brain dump, to get this saved in some places safer than
the compile farm or (worse) my head ;-)

Patch hide | download patch | download mbox

Index: optabs.c
===================================================================
--- optabs.c    (revision 193394)
+++ optabs.c    (working copy)
@@ -170,9 +170,9 @@  optab_libfunc (optab optab, enum machine

    If the last insn does not set TARGET, don't do anything, but return 1.

-   If a previous insn sets TARGET and TARGET is one of OP0 or OP1,
-   don't add the REG_EQUAL note but return 0.  Our caller can then try
-   again, ensuring that TARGET is not one of the operands.  */
+   If the last insn or a previous insn sets TARGET and TARGET is one of OP0
+   or OP1, don't add the REG_EQUAL note but return 0.  Our caller can then
+   try again, ensuring that TARGET is not one of the operands.  */

 static int
 add_equal_note (rtx insns, rtx target, enum rtx_code code, rtx op0, rtx op1)
@@ -192,6 +192,12 @@  add_equal_note (rtx insns, rtx target, e
   if (GET_CODE (target) == ZERO_EXTRACT)
     return 1;

+  /* If TARGET is in OP0 or OP1, punt.  We'd end up with a note referencing
+     a value changing in the insn, so the note would be invalid for CSE.  */
+  if (reg_overlap_mentioned_p (target, op0)
+      || (op1 && reg_overlap_mentioned_p (target, op1)))
+    return 0;
+
   for (last_insn = insns;
        NEXT_INSN (last_insn) != NULL_RTX;
        last_insn = NEXT_INSN (last_insn))
@@ -207,21 +213,6 @@  add_equal_note (rtx insns, rtx target, e
          || ! rtx_equal_p (XEXP (SET_DEST (set), 0), target)))
     return 1;

-  /* If TARGET is in OP0 or OP1, check if anything in SEQ sets TARGET
-     besides the last insn.  */
-  if (reg_overlap_mentioned_p (target, op0)
-      || (op1 && reg_overlap_mentioned_p (target, op1)))
-    {
-      insn = PREV_INSN (last_insn);
-      while (insn != NULL_RTX)
-       {
-         if (reg_set_p (target, insn))
-           return 0;
-
-         insn = PREV_INSN (insn);
-       }
-    }
-
   if (GET_RTX_CLASS (code) == RTX_UNARY)
     switch (code)
       {