[dataflow] Re: Dataflow branch review #1
Kenneth Zadeck
zadeck@naturalbridge.com
Thu May 31 01:05:00 GMT 2007
Ian Lance Taylor wrote:
> I've started looking at the overall dataflow patch. Here is my first
> set of comments, covering auto-inc-dec.c.
>
> Ian
>
>
> In auto-inc-dec.c:
>
> Update copyright date.
>
>
> +enum form
> + {
> +/* This pass was originally removed from flow.c. However there is
> + almost nothing that remains of that code.
> +
> + There are (4) basic forms that are matched:
> +
> +*/
> + FORM_PRE_ADD,
> +/*
> + a <- b + c
> + ...
> + *a
> +
> + becomes
> +
> + a <- b
> + ...
> + *(a += c) pre
> +*/
>
> Please move the comments for the enum types before the enum
> definition. gcc practice is to put comments before the object being
> described. So the comment above should come before FORM_PRE_ADD.
>
> Also, indent as
>
> enum form
> {
> FORM_PRE_ADD
> };
>
>
> + The arrays are not cleared when we move from block to block so
> + whenever an insn is retrieved from these arrays, it's block number
> + must be comared with the current block.
> +*/
>
> Typo: "comared" => "compared"
>
> +static rtx * reg_next_use = NULL;
> +static rtx * reg_next_inc_use = NULL;
> +static rtx * reg_next_def = NULL;
>
> Just write "rtx*", not "rtx *".
>
>
> + if ((REG_NOTE_KIND(note) == REG_DEAD)
> + && pattern == XEXP (note, 0))
>
> Space after REG_NOTE_KIND.
>
> + else prev_note = note;
>
> Put "prev_note = note;" on the next line.
>
>
> +/* Change mem_insn.mem_loc so that uses NEW_ADDR that increments
> + INC_REG. To have reached this point, the change is a legitimate
> + one from a dataflow point of view. The only questions are is this
> + a valid change to the instruction and is this a profitable change
> + to the instruction. */
> +
> +static bool
> +attempt_change (rtx new_addr, rtx inc_reg)
>
> I don't understand the first sentence in the comment.
>
> + reg_next_def [regno] = mov_insn;
> + reg_next_use [regno] = NULL;
>
> No space before '[' here and elsewhere.
>
> + switch (inc_insn.form)
> ...
> + case FORM_last:
> + gcc_unreachable ();
>
> Add a default: case which calls gcc_unreachable.
>
> + /* Recompute the df info for the insns that have changed. */
> + delete_insn (inc_insn.insn);
>
> This comment looks wrong.
>
>
> In try_merge:
>
> + /* The width of the mem being access. */
>
> Typo: "access" => "accessed".
>
> + switch (inc_insn.form)
> ...
> + case FORM_last:
> + gcc_unreachable ();
>
> Add a default case here also.
>
>
> +/* Return the next insn that uses (if reg_next_use is passed in
> + NEXT_ARRAY) or defines if reg_next_def is passed in NEXT_ARRAY)
> + REGNO in BB. */
>
> Missing left parenthesis before "if reg_next_def", I think.
>
>
> + This function is called in two contexts, if BEFORE_MEM is true,
> + this is called for each insn in the basic block. If BEFORE_MEM is
> + false, it is called for the instruction in the block that uses the
> + index register for som memory reference that is currently being
> + processed. */
>
> Typo: "som" => "some".
>
>
> + In the case where the MEM_)INSN has two registers in the reference,
> + this function may be called recursively. The first time looking
> + for an add of the first register, and if that fails, looking for an
> + add of the second register. The FIRST_TRY parameter is used to
> + only allow the parameters to be reversed once. */
>
> Extraneous ')' in first line.
>
As it has been commanded, so it has been done!!!
I took the general comments and applied them to the entire diff for the
branch.
2007-05-30 Kenneth Zadeck <zadeck@naturalbridge.com>
* auto-inc-dec.c: Updated copyright date.
(enum form, set_inc_state, dump_inc_insn, move_dead_notes,
insert_move_insn_before, attempt_change, try_merge, find_address,
find_mem): Reformatted.
(reverse_inc, find_address): Fixed spelling.
(attempt_change, try_merge): Add default case.
* basic-block.h: Updated copyright date.
* bitmap.c: Updated copyright date.
* bitmap.h: Updated copyright date.
* cfganal.c: Updated copyright date.
* cfg.c: Updated copyright date.
* cfghooks.h: Updated copyright date.
* cfglayout.c: Updated copyright date.
* cfgloop.c: Updated copyright date.
* cfgloop.h: Updated copyright date.
* cfgrtl.c: Updated copyright date.
* combine.c: Updated copyright date.
* combine-stack-adj.c: Updated copyright date.
* config/arc/arc.c: Updated copyright date.
* config/arm/arm.c: (use_return_insn,
arm_compute_save_reg0_reg12_mask, arm_get_frame_offsets,
arm_save_coproc_regs): Fixed formatting.
* config/bfin/bfin.c: Updated copyright date.
* config/c4x/c4x.c: Updated copyright date.
* config/c4x/c4x.h: Updated copyright date.
* config/cris/cris.c: Updated copyright date.
* config/crx/crx.c: Updated copyright date.
* config/crx/crx.h: Updated copyright date.
* config/darwin.c: Updated copyright date.
* config/frv/frv.c: Updated copyright date.
* config/h8300/h8300.c: Updated copyright date.
* config/h8300/h8300.md: Updated copyright date.
* config/ia64/ia64.h: Updated copyright date.
* config/iq2000/iq2000.c: Updated copyright date.
* config/iq2000/iq2000.h: Updated copyright date.
* config/m32c/m32c.c: Updated copyright date.
* config/m68hc11/m68hc11.c: Updated copyright date.
* config/m68k/m68k.c: Updated copyright date.
* config/mips/mips.c: Updated copyright date.
* config/mips/mips.md: Updated copyright date.
* config/mmix/mmix.c: Updated copyright date.
* config/mn10300/mn10300.c: Updated copyright date.
* config/mt/mt.c: Updated copyright date.
(mt_print_operand_simple_address, mt_print_operand): Fixed formatting.
* config/mt/mt.h: Updated copyright date and fixed formatting.
* config/pa/pa.c: Updated copyright date.
* config/pa/pa.h: Updated copyright date.
* config/pdp11/pdp11.c: Updated copyright date.
* config/pdp11/pdp11.h: Updated copyright date.
* config/rs6000/predicates.md: Updated copyright date.
* config/s390/s390.c: Updated copyright date.
* config/score/score-mdaux.c: Updated copyright date.
* config/sh/sh.c: Updated copyright date.
* config/sh/sh.md: Updated copyright date.
* config/sparc/sparc.c: Updated copyright date.
* config/stormy16/stormy16.c: Updated copyright date.
* config/v850/v850.c: Updated copyright date.
* config/vax/vax.c: Updated copyright date.
* cselib.c: Updated copyright date.
(expand_loc): Fixed formatting.
* cselib.h: Updated copyright date.
* dbgcnt.c: Updated copyright date.
* dbgcnt.def: Updated copyright date.
* dbgcnt.h: Updated copyright date.
* dce.c: Updated copyright date.
(fast_dce): Fixed formatting.
* dce.h: Updated copyright date.
* ddg.c: Updated copyright date.
* ddg.h: Updated copyright date.
* df-core.c: Updated copyright date.
* df.h: Updated copyright date and fixed formatting.
* doc/cfg.texi: Updated copyright date.
* doc/rtl.texi: Updated copyright date.
* dominance.c: Updated copyright date.
* function.h: Updated copyright date.
* fwprop.c: Updated copyright date.
* global.c: Updated copyright date.
* integrate.c: Updated copyright date.
* local-alloc.c: Updated copyright date.
* loop-init.c: Updated copyright date.
* loop-invariant.c: Updated copyright date.
* loop-iv.c: Updated copyright date.
* optabs.h: Updated copyright date.
* output.h: Updated copyright date.
* postreload.c: Updated copyright date.
* postreload-gcse.c: Updated copyright date.
* recog.h: Updated copyright date.
* regmove.c: Updated copyright date.
* reg-notes.def: Updated copyright date.
* regrename.c: Updated copyright date.
* reg-stack.c: Updated copyright date.
* reload.c: Updated copyright date.
* reorg.c: Updated copyright date.
* resource.c: Updated copyright date.
* resource.h: Updated copyright date.
* rtl-factoring.c: Updated copyright date.
* sbitmap.c: Updated copyright date.
* sbitmap.h: Updated copyright date.
* sched-deps.c: Updated copyright date.
* sched-ebb.c: Updated copyright date.
* sched-int.h: Updated copyright date.
* sched-rgn.c: Updated copyright date.
* sched-vis.c: Updated copyright date.
* see.c: Updated copyright date.
(see_handle_relevant_uses): Fixed formatting.
* stack-ptr-mod.c: Updated copyright date.
* struct-equiv.c: Updated copyright date.
* tracer.c: Updated copyright date.
* web.c: Updated copyright date.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: iant1.diff
Type: text/x-patch
Size: 59389 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20070531/842383be/attachment.bin>
More information about the Gcc-patches
mailing list