Summary: | [killloop-branch] code motion of non-invariant expressions with hard registers. | ||
---|---|---|---|
Product: | gcc | Reporter: | Steven Bosscher <steven> |
Component: | rtl-optimization | Assignee: | Zdenek Dvorak <rakdver> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | gcc-bugs, pinskia |
Priority: | P3 | Keywords: | EH, wrong-code |
Version: | unknown | ||
Target Milestone: | --- | ||
Host: | Target: | ia64 | |
Build: | Known to work: | ||
Known to fail: | Last reconfirmed: | 2005-11-11 17:52:46 | |
Bug Depends on: | |||
Bug Blocks: | 22366, 24408 | ||
Attachments: |
loop2_init dump
loop2_invariant dump Fix df-scan.c |
Description
Steven Bosscher
2005-11-09 20:11:26 UTC
Created attachment 10197 [details]
loop2_init dump
Created attachment 10198 [details]
loop2_invariant dump
This is not an ia64 specific issue as far as I can see, on x86_64, we get: (note 64 61 62 2 [bb 2] NOTE_INSN_BASIC_BLOCK) (insn 62 64 63 2 (set (reg:DI 63) (reg:DI 0 ax)) -1 (nil) (nil)) (insn 63 62 53 2 (set (reg:DI 65) (reg:DI 1 dx)) -1 (nil) (nil)) so those could also be moved above the loop also (if you change the cost). The sets come from dw2_build_landing_pads: emit_move_insn (cfun->eh->exc_ptr, gen_rtx_REG (ptr_mode, EH_RETURN_DATA_REGNO (0))); emit_move_insn (cfun->eh->filter, gen_rtx_REG (targetm.eh_return_filter_mode (), EH_RETURN_DATA_REGNO (1))); I am testing the following patch: Index: except.c =================================================================== *** except.c (revision 106702) --- except.c (working copy) *************** dw2_build_landing_pads (void) *** 1527,1543 **** /* If the eh_return data registers are call-saved, then we won't have considered them clobbered from the call that ! threw. Kill them now. */ for (j = 0; ; ++j) { unsigned r = EH_RETURN_DATA_REGNO (j); if (r == INVALID_REGNUM) break; ! if (! call_used_regs[r]) ! { ! emit_insn (gen_rtx_CLOBBER (VOIDmode, gen_rtx_REG (Pmode, r))); ! clobbers_hard_regs = true; ! } } if (clobbers_hard_regs) --- 1527,1544 ---- /* If the eh_return data registers are call-saved, then we won't have considered them clobbered from the call that ! threw. But we need to clobber all hard registers, since ! df.c assumes that call clobbers are not definitions ! (normally it is invalid to use call-clobbered register ! after the call). */ for (j = 0; ; ++j) { unsigned r = EH_RETURN_DATA_REGNO (j); if (r == INVALID_REGNUM) break; ! ! emit_insn (gen_rtx_CLOBBER (VOIDmode, gen_rtx_REG (Pmode, r))); ! clobbers_hard_regs = true; } if (clobbers_hard_regs) 1. Call clobbers aren't definitions. They are clobbers (IE kills). They do not generate a new value, they simply specify the old value is dead. Thus, you don't mean to say "df.c assumes call clobbers ...", because it's not an assumption. 2. The patch is actually an incorrect workaround. The real problem is that df.c on mainline doesn't get reaching definitions right for a number of reasons. For example, explicit clobbers in CALL_INSN_FUNCTION_USAGE but *not* in regs_invalidated_by_call will be treated as gen's, because they don't fall into the special case in df_bb_rd_local_compute. But they are not gen's. Have you tried making sure CLOBBER's don't get into the rd_gen set? I imagine that will fix your bug. There are also a ton of other bugs related to hard regs in reaching definitions on df.c in mainline, i'm just guessing. Try the df.c from the dataflow-branch, and see if it fixes the bug, because honestly, i think you are barking up the wrong tree here. Clobbers are most *certainly* not new definitions. Subject: Re: [killloop-branch] code motion of non-invariant expressions with hard registers.
> ------- Comment #6 from dberlin at gcc dot gnu dot org 2005-11-09 22:53 -------
> 1. Call clobbers aren't definitions. They are clobbers (IE kills). They do not
> generate a new value, they simply specify the old value is dead. Thus, you
> don't mean to say "df.c assumes call clobbers ...", because it's not an
> assumption.
>
>
> 2. The patch is actually an incorrect workaround. The real problem is that
> df.c on mainline doesn't get reaching definitions right for a number of
> reasons.
>
> For example, explicit clobbers in CALL_INSN_FUNCTION_USAGE but *not* in
> regs_invalidated_by_call will be treated as gen's, because they don't fall into
> the special case in df_bb_rd_local_compute. But they are not gen's.
>
> Have you tried making sure CLOBBER's don't get into the rd_gen set?
>
> I imagine that will fix your bug.
I am fairly sure you are wrong, in this case. The code is like
bb1:
call (throw);
bb2:
reg = r15
where r15 is set by exception handling. The problem is that there
currently simply is no insn that defines r15.
Okay, well, there are also bugs in df.c on mainline in regards to not creating uses for "always-live" registers and registers that are live over eh edges, for example. (It does none of this). Seriously. Hard regs are very badly broken in mainline df.c in terms of liveness and use-def/def-use chains. Steven tells me flow.c gets liveness for your particular example wrong too though. In the dataflow branch df.c, we would add a use for the "live on entry to eh" regs, in the artifical uses code for eh blocks, and it would just work. Actually, flow.c does get it right. From t.C.26.life1 (at -O1 -fno-move-loop-invariants): ;; Start of basic block 3, registers live: 0 [ap] 1 [r1] 12 [r12] 15 [r15] 16 [r16] 328 [sfp] 341 344 (code_label/s 62 30 65 3 8 "" [1 uses]) (note 65 62 63 3 [bb 3] NOTE_INSN_BASIC_BLOCK) (insn 63 65 64 3 (set (reg:DI 348) (reg:DI 15 r15)) 5 {*movdi_internal} (nil) (expr_list:REG_DEAD (reg:DI 15 r15) (nil))) (insn 64 63 54 3 (set (reg:DI 350) (reg:DI 16 r16)) 5 {*movdi_internal} (nil) (expr_list:REG_DEAD (reg:DI 16 r16) (nil))) Subject: Re: [killloop-branch] code motion of
non-invariant expressions with hard registers.
On Wed, 2005-11-09 at 23:45 +0000, steven at gcc dot gnu dot org wrote:
>
> ------- Comment #10 from steven at gcc dot gnu dot org 2005-11-09 23:45 -------
> Actually, flow.c does get it right.
Okay, then df.c on dataflow branch should get it right too.
Confirmed. The dataflow-branch is also failing for this test case. There is nothing in the df object that makes loop-invariant.c think that the insns setting those EH_RETURN_DATA_REGNO regs are not loop invariant. Send me a dump of df_analyze (df, -1, DF_LR | DF_HARD_REGS | DF_ARTIFICIAL_USES), and point out what you say is wrong and i'll make it right. Subject: Bug 24762 Author: rakdver Date: Tue Nov 15 14:27:48 2005 New Revision: 107017 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=107017 Log: PR rtl-optimization/24762 * except.c (dw2_build_landing_pads): Always emit clobbers for eh registers. Modified: branches/killloop-branch/gcc/ChangeLog.killloop branches/killloop-branch/gcc/except.c This is still not fixed on mainline, even with the new df implementation. The code to record them is not executed (at least, not on AMD64). The function responsible for recording them is df_bb_refs_record, specifically this code: ifdef EH_RETURN_DATA_REGNO if ((df->flags & DF_HARD_REGS) && df_has_eh_preds (bb)) { unsigned int i; /* Mark the registers that will contain data for the handler. */ if (current_function_calls_eh_return) for (i = 0; ; ++i) { unsigned regno = EH_RETURN_DATA_REGNO (i); if (regno == INVALID_REGNUM) break; df_ref_record (dflow, regno_reg_rtx[i], ®no_reg_rtx[i], bb, NULL, DF_REF_REG_DEF, DF_REF_ARTIFICIAL | DF_REF_AT_TOP, false); } } #endif But current_function_calls_eh_return is false for the test case, even though the EH_RETURN_DATA_REGNO regs are set: Breakpoint 3, df_bb_refs_record (dflow=0x11203f0, bb=0x2aaaaafaa780) at df-scan.c:1519 1519 if ((df->flags & DF_HARD_REGS) (gdb) p debug_bb(bb) ;; basic block 5, loop depth 1, count 0 ;; prev block 4, next block 6 ;; pred: 4 (ab,abcall,eh) ;; succ: 7 [29.0%] 6 [71.0%] (fallthru,loop_exit) ;; Registers live at start: (nil) (code_label/s 61 28 64 5 8 "" [1 uses]) (note 64 61 62 5 [bb 5] NOTE_INSN_BASIC_BLOCK) (insn 62 64 63 5 (set (reg:DI 65) (reg:DI 0 ax)) 81 {*movdi_1_rex64} (nil) (nil)) (insn 63 62 53 5 (set (reg:DI 66) (reg:DI 1 dx)) 81 {*movdi_1_rex64} (nil) (nil)) (insn 53 63 54 5 (set (reg:CCZ 17 flags) (compare:CCZ (reg:DI 66) (const_int 1 [0x1]))) 2 {cmpdi_1_insn_rex64} (nil) (nil)) (jump_insn 54 53 65 5 (set (pc) (if_then_else (eq (reg:CCZ 17 flags) (const_int 0 [0x0])) (label_ref 30) (pc))) 511 {*jcc_1} (nil) (expr_list:REG_BR_PROB (const_int 2900 [0xb54]) (nil))) ;; Registers live at end: (nil) $5 = void (gdb) next 1524 if (current_function_calls_eh_return) (gdb) p cfun->calls_eh_return $6 = 0 As a result, we still consider insns 62 and 63 to be loop invariant: ;; Function void test01(int) (_Z6test01i) Set in insn 22 is invariant (0), cost 0, depends on Set in insn 25 is invariant (1), cost 0, depends on Set in insn 26 is invariant (2), cost 0, depends on Set in insn 62 is invariant (3), cost 4, depends on Set in insn 63 is invariant (4), cost 4, depends on Set in insn 33 is invariant (5), cost 0, depends on 3 Created attachment 10642 [details]
Fix df-scan.c
There doesn't seem to be a good reason to make adding the artificial defs for the EH_RETURN_DATA_REGNO regs conditional on current_function_calls_eh_return.
Subject: Bug 24762 Author: zadeck Date: Fri Jan 27 22:23:32 2006 New Revision: 110312 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110312 Log: 2006-01-27 Daniel Berlin <dberlin@dberlin.org> Kenneth Zadeck <zadeck@naturalbridge.com> PR rtl-optimization/24762 * doc/tm.texi: Added TARGET_EXTRA_LIVE_ON_ENTRY. * targhooks.c (hook_void_bitmap): New hook prototype. * targhoohs.h (hook_void_bitmap): Ditto. * bitmap.h (bitmap_head_def): Moved to coretypes.h. * coretypes.h (bitmap_head_def): Moved from bitmap.h. * target.h (live_on_entry): New function pointer. * df-scan.c (df_all_hard_regs): Removed. (df_scan_dump, df_hard_reg_init): Removed df_all_hard_regs. (df_scan_free_internal): Added df->entry_block_defs. (df_scan_alloc): Ditto. (df_scan_dump): Ditto. (df_uses_record): Plumbed flag field properly thru calls. Record EH_RETURN_DATA_REGNO in eh blocks unconditionally. This part fixes PR24762. (df_bb_refs_record): Added code to make the frame and arg pointers live in EH blocks. (df_refs_record): Added call to df_record_entry_block_defs. (df_record_entry_block_defs): New function. * df-core.c: Added comments to describe new artifical defs. * df.h (DF_REF_DIES_AFTER_THIS_USE): New flag in enum df_ref_flags. (entry_block_defs): New field in struct df. (df_all_hard_regs): Deleted. * target-def.h: Added TARGET_EXTRA_LIVE_ON_ENTRY. * df-problems.c (df_ru_bb_local_compute_process_def): Added code to handle artifical defs in the entry to a function. (df_ru_bb_local_compute): Ditto. (df_rd_bb_local_compute_process_def): Ditto. (df_rd_bb_local_compute): Ditto. (df_lr_bb_local_compute): Ditto. (df_ur_bb_local_compute): Ditto. (df_urec_bb_local_compute): Ditto. (df_chain_create_bb): Ditto. (df_ur_local_finalize): Removed entry. (df_urec_init): Ditto. (df_urec_local_finalize): Ditto. (df_ri_bb_compute): Added detection of last use of pseudos. * Makefile.in (df-scan.o): Updated dependencies. * config/mips/mips-protos.h (mips_set_live_on_entry): Added. * config/mips/mips.c (mips_set_live_on_entry): Added. * config/mips/mips.c (TARGET_EXTRA_LIVE_ON_ENTRY): Added value for target hook. * dce.c (marked_insn_p): Added code to handle artifical defs. Modified: trunk/gcc/ChangeLog trunk/gcc/Makefile.in trunk/gcc/bitmap.h trunk/gcc/config/mips/mips-protos.h trunk/gcc/config/mips/mips.c trunk/gcc/coretypes.h trunk/gcc/df-core.c trunk/gcc/df-problems.c trunk/gcc/df-scan.c trunk/gcc/df.h trunk/gcc/doc/tm.texi trunk/gcc/target-def.h trunk/gcc/target.h trunk/gcc/targhooks.c trunk/gcc/targhooks.h Subject: Bug 24762 Author: rakdver Date: Thu Feb 9 22:34:23 2006 New Revision: 110815 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110815 Log: PR rtl-optimization/24762 * df-scan.c (df_bb_refs_record): Record correct registers defined on eh edges. Modified: trunk/gcc/ChangeLog trunk/gcc/df-scan.c Fixed. |