This bug was discovered while adapting MSP430 port for GCC 4.x and is
persistent at least in both GCC 4.2.4 and 4.3.4.
This description consists of a problem description, a detailed reason analysis
pointing out exact GCC functions, and a proposed solution.
---------------------------------------------------------------
1. Problem
Compiling the following code for MSP430 (16-bit RISC) with maximum optimization
(-O2) causes an "internal error int redirect_branch_edge()" failure:
-------------
for (volatile long t = 0; t < 1024; t++) ;
-------------
The problem is related to the "cbranchsi" instruction, that clobbers one of
compared registers. Here is a quote from MD file:
-------------
(define_insn "*cbranchsi_others"
  [(parallel [(set (pc)
     (if_then_else (match_operator:SI 1 "inequality_operator"
                    [(match_operand:SI 2 "register_operand" "r")
                     (match_operand:SI 3 "general_operand" "rmi")])
      (label_ref (match_operand 0 "" ""))
      (pc)))
  (clobber (match_dup 2))])] ;<========== Attention! clobber (match_dup) !!!
-------------
2. Reason
2.1 The "*cbranchsi_others" INSN clobbers one of its arguments [clobber
(match_dup 2)]. During loop optimization, the source register (operand 2) gets
changed, while the register reference in CLOBBER attribute does not. After such
change, instruction is no longer recognizable according to machine decsription,
and the compilation fails.
2.2 The move_invariant_reg() function in loop-invariant.c performs register
renaming based on "register use lists". When the "cbranchsi_others" instruction
gets scanned for register uses, the CLOBBER use does not get recorded, and
thus, does not get changed later, resulting in an erroneous INSN pattern:
------------- Before move_invariant_reg() (insn_invalid_p(insn) = 0)
-------------
(parallel [
        (set (pc)
            (if_then_else (ge:SI (reg:SI 30)
                    (reg:SI 24 [ j.1 ]))
                (label_ref:HI 34)
                (pc)))
        (clobber:SI (reg:SI 30))
    ])
------------- After move_invariant_reg() (insn_invalid_p(insn) = 0)
-------------
(parallel [
        (set (pc)
            (if_then_else (ge:SI (reg:SI31) <===== CHANGED 30 to 31
                    (reg:SI 24 [ j.1 ]))
                (label_ref:HI 34)
                (pc)))
        (clobber:SI (reg:SI 30))  <======= NOT CHANGED 30 TO 31
    ])
-------------
Obviously, the modification makes the instruction invalid, as it uses and
clobbers different registers, and thus, no existing INSN pattern matches it.
2.3 The "register use lists" are built using the record_uses() function in
loop-invariant.c based on DF use records. The record_uses() function considers
DF_INSN_USES() and DF_INSN_EQ_USES() lists (only DF_INSN_USES in gcc 4.2.4).
CLOBBERed registers are put into DF_INSN_DEFS() list, that is not considered in
record_uses(). That way, a CLOBBER attribute remains unchanged, when used
registers are renamed.
2.4 The df_defs_record() function in df-scan.c places both SET and CLOBBER
references into DF_INSN_DEFS() list:
  if (code == SET || code == CLOBBER)
    {
      //...
      df_def_record_1 (collection_rec, x, bb, insn, clobber_flags);
    }
2.5 Summary. Invariant moving optimization in loop-invariant.c disrupts INSN
patterns that USE and CLOBBER the same register, making them invalid.
-------------
3. Proposed solution.
3.1. For my needs, I have patched loop-invariant.c by adding the following
function to explicitly record all CLOBBER references from INSN that match USE
references:
static void record_clobber_uses_workaround(rtx insn, struct invariant *inv,
struct df_ref *use)
{
        rtx pat = PATTERN(insn);
        if (GET_CODE(pat) == PARALLEL)
        {
                int len = XVECLEN(pat, 0);
                int idx = 0;

                for (idx = 0; idx < len; idx++)
                {
                        rtx subexp = XVECEXP(pat, 0, idx);
                        if (GET_CODE(subexp) == CLOBBER)
                        {
                                if (XEXP (subexp, 0) == *DF_REF_REAL_LOC (use))
                                        record_use (inv->def, &XEXP (subexp,
0), DF_REF_INSN (use));
                        }                               
                }
        }
}
The record_uses() function got patched accordingly:
 if (inv)
+  {
        record_use (inv->def, DF_REF_REAL_LOC (use), DF_REF_INSN (use));
+       record_clobber_uses_workaround(insn, inv, use);         
+  }
3.2. I would recommend re-analyzing the more common case, when a single
instruction uses and defines the same register, and decide, whether loop
optimizations in loop-invariant.c should move such register. The bug does not
seem to affect most architectures, however, specific INSN patterns, as
described above, do trigger it. Maybe, a common function returning the list of
all "match_dup"-ed references for a given INSN and an operand to detect such
cases in a uniform way.


-- 
           Summary: move_invariant_reg() damages CBRANCH instructions with
                    CLOBBER attribute
           Product: gcc
           Version: 4.3.4
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: regression
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: shcherbakov at daad-alumni dot de
 GCC build triplet: independent (i686-cygwin)
  GCC host triplet: independent (i686-cygwin)
GCC target triplet: msp430 (see below)


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41190

Reply via email to