Missing barrier in outof_cfglayout

Georg-Johann Lay avr@gjlay.de
Mon May 11 17:37:00 GMT 2015


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




More information about the Gcc mailing list