This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Missing barrier in outof_cfglayout


Am 05/12/2015 um 05:13 PM schrieb Jeff Law:
On 05/12/2015 08:58 AM, Georg-Johann Lay wrote:
Ah, yes.  The ICE is actually in verify_flow_info: "wrong number of
branch edges after unconditional jump in bb 4".

It starts with an almost trivial jump table:


(jump_insn 82 81 83 19 (parallel [
             (set (pc)
                 (reg/f:SI 60))
             (use (label_ref 83))
         ])
      (nil)
  -> 83)
;;  succ:       20 [50.0%]
;;              31 [50.0%]

;; Insn is not within a basic block
(code_label 83 82 84 15 "" [3 uses])
;; Insn is not within a basic block
(jump_table_data 84 83 85 (addr_vec:SI [
             (label_ref:SI 130)
             (label_ref:SI 86)
             (label_ref:SI 130)
             (label_ref:SI 130)
         ]))

At .cse1, cse.c:cse_insn executes

       /* We don't normally have an insn matching (set (pc) (pc)), so
          check for this separately here.  We will delete such an
          insn below.

          For other cases such as a table jump or conditional jump
          where we know the ultimate target, go ahead and replace the
          operand.  While that may not make a valid insn, we will
          reemit the jump below (and also insert any necessary
          barriers).  */
       if (n_sets == 1 && dest == pc_rtx
           && (trial == pc_rtx
           || (GET_CODE (trial) == LABEL_REF
               && ! condjump_p (insn))))
         {
           /* Don't substitute non-local labels, this confuses CFG.  */
           if (GET_CODE (trial) == LABEL_REF
           && LABEL_REF_NONLOCAL_P (trial))
         continue;

           SET_SRC (sets[i].rtl) = trial;
           cse_jumps_altered = true;
           break;
         }

on the jump insn and with trial = (label_ref 83).

The code pointed out by Jeff then replaces the original jump insn by

(jump_insn 139 81 82 16 (set (pc)
         (label_ref 83)) bug.c:33 -1
      (nil))

That's obviously nonsense as it jumps to the jump_table_data!
It's likely nonsensical.  You'd have to figure out why TRIAL has that seemingly


hmmm, it would actually lead to valid machine code: The jump table is actually a dispatch table, i.e. it contains a list of direct jumps to the case labels. casesi loads the value of the label associated with the jump table into a register, adds an offset, and then jumps to that address (which in turn dispatches to the very cases via a direct jump).

In this specific case cse finds out that the offset (reg 59 below) is always 0, i.e. the offset computation will always yield the value of the label associated to the jump table (code_label 84), and then jumps to that label.

When expanded, casesi will yield something like

(note 138 77 78 19 [bb 19] NOTE_INSN_BASIC_BLOCK)

(insn 78 138 79 19 (set (reg:SI 61)
                        (high:SI (label_ref:SI 84))))

(insn 79 78 80 19 (set (reg:SI 60)
                       (lo_sum:SI (reg:SI 61)
                                  (label_ref:SI 84))))

(insn 80 79 81 19 (set (reg:SI 59)
                       (mult:SI (reg:SI 59)
                                (const_int 4))))

(insn 81 80 82 19 (set (reg:SI 60)
                       (plus:SI (reg:SI 60)
                                (reg:SI 59))))

(jump_insn 82 81 83 19 (parallel [(set (pc)
                                       (reg:SI 60))
                                  (use (label_ref 84))])
 -> 84)
;;  succ:       20 [50.0%]
;;              31 [50.0%]

(barrier 83 82 84)

;; Insn is not within a basic block
(code_label 84 83 85 15 "" [3 uses])

;; Insn is not within a basic block
(jump_table_data 85 84 86 (addr_vec:SI [
            (label_ref:SI 132)
            (label_ref:SI 87)
            (label_ref:SI 132)
            (label_ref:SI 132)
        ]))
(barrier 86 85 87)


bogus value.  I would have expected it to have one of the entries from the
ADDR_VEC.

Other backends are also using dispatch tables and are jumping into the table, and internal docs for casesi read: "Instruction to jump through a dispatch table"...

Wrapping the tablejump's register (reg 60 above) into UNSPEC fixes the problem, but just because there is no insn to load such an unspec into a register and hence cse does not perform respective optimization.

But even with the right value in TRIAL, we shouldn't be slamming on the SET_SRC
of the jump, but instead should be using the routines from cfgrtl.c to
manipulate the CFG and RTL datastructures properly.

jeff

I am having the trouble with 4.9, but the current trunk still contains the same sequence...

Given the routines from from cfgrtl.c were used: would it then be legal to jump to a label which is not contained in any basic block?

Johann


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]