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/11/2015 um 06:58 PM schrieb Jeff Law:

Wow, that was fast!

On 05/11/2015 10:19 AM, Georg-Johann Lay wrote:
When pass outof_cfglayout is adding barriers, it appears that it misses
some situations and then runs into "ICE: missing barrier" in the
remainder (or, with checking disabled, into some other assertion).

[ ... ]

The last else is entered for an UNconditional jump with e_fall = 0 so
that after the unconditional jump_insn no barrier is emit.

Now I am unsure about the right condition when to add the missing
barrier; the following change works, but presumably the condition is
only 99% correct ;-)
But why is there a JUMP_INSN here to start with?   Prior to cfglayout I believe
simple unconditional jumps shouldn't exist in the INSN chain. Instead they are
inserted as we leave cfglayout based on the state of the CFG.

The respective BB has just that unconditional jump_insn; it is generated
by CSE1 as it optimized a switch statement and was originally some
conditional jump (-fno-jump-tables) or computed jump (with -fjump-tables):
This is where I'd focus my efforts.  Instead of modifying the JUMP_INSN
directly, we're supposed to be going through the various CFG routines.

Most likely it's this code:
      /* If this SET is now setting PC to a label, we know it used to
          be a conditional or computed branch.  */
       else if (dest == pc_rtx && GET_CODE (src) == LABEL_REF
                && !LABEL_REF_NONLOCAL_P (src))
...

              new_rtx = emit_jump_insn_before (gen_jump (XEXP (src, 0)), insn);
               JUMP_LABEL (new_rtx) = XEXP (src, 0);
               LABEL_NUSES (XEXP (src, 0))++;

Yet, it actually is!

Before cse1 (dfinit):

;; basic block 19, loop depth 0, count 0, freq 4623, maybe hot
;;  prev block 18, next block 20, flags: (RTL, MODIFIED)
;;  pred:       18 [89.7%]  (FALLTHRU)
;; bb 19 artificial_defs: { }
;; bb 19 artificial_uses: { u-1(26){ }u-1(30){ }u-1(32){ }}
(note 172 110 111 19 [bb 19] NOTE_INSN_BASIC_BLOCK)
(jump_insn 111 172 173 19 (set (pc)
        (if_then_else (ltu (reg:SI 51 [ D.1451 ])
                (const_int 1 [0x1]))
            (label_ref 133)
            (pc))) bug.c:33 140 {*bltu}
     (int_list:REG_BR_PROB 1394 (nil))
 -> 133)
;;  succ:       25 [13.9%]
;;              20 [86.1%]  (FALLTHRU)

And in cse1, following jumps:

deferring deletion of insn with uid = 111.
Purged edges from bb 19

...

;; basic block 19, loop depth 0, count 0, freq 4623, maybe hot
;; Invalid sum of incoming frequencies 5156, should be 4623
;;  prev block 18, next block 20, flags: (REACHABLE, RTL, MODIFIED)
;;  pred:       18 [100.0%]  (FALLTHRU)
(note 172 109 180 19 [bb 19] NOTE_INSN_BASIC_BLOCK)
(jump_insn 180 172 117 19 (set (pc)
        (label_ref 133)) bug.c:33 -1
     (nil)
 -> 133)
;;  succ:       23 [100.0%]


And it almost certainly should be calling into the cfgrtl.c routines instead.

You mean something like try_redirect_by_replacing_jump?


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?


Johann



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