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

login
register
mail settings
Submitter Steven Bosscher
Date Nov. 27, 2012, 11:53 p.m.
Message ID <CABu31nPOwDmsvsD8Ge42n0G2eFxHhm9WWcAyx8ku8sChDnaXHQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/202324/
State New
Headers show

Comments

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

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)
       {