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


On 05/12/2015 08:58 AM, Georg-Johann Lay wrote:
Am 05/11/2015 um 10:43 PM schrieb Steven Bosscher:
On Mon, May 11, 2015 at 7:37 PM, Georg-Johann Lay wrote:
BTW, what's the policy about unconditional jumps at that time?  There
are
plenty of unconditional jumps around and all are legitimate; just
this one
generated by cse1 is wrong?

If you're in cfglayout mode, then there should be no unconditional
jumps to labels. The only JUMP_INSNs should be unconditional,
computed, or return jumps.

If you do have unconditional jumps at this stage, something is
seriously broken.

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 bogus value. I would have expected it to have one of the entries from the ADDR_VEC.

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


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