Hi, Bernd Thanks for the review.
> > You might want to split it up if there are several logically independent > pieces. I can't quite make sense of it all, and I'm not too familiar with SMS > anyway, so the following is not a complete review, just a selection of issues > I observed. Ok, I have split the patch in several pieces by following link. https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00377.html > There are a large number of formatting and style problems. I'll be pointing > out some instances, but please read > http://www.gnu.org/prep/standards/standards.html#Writing-C > and self-review your patch before resubmission. I have rewrite most the the comments. Please correct me if i still missing something. >> - /* We do not handle setting only part of the register. */ >> - if (DF_REF_FLAGS (adef) & DF_REF_READ_WRITE) >> - return GRD_INVALID; >> - > > > Why this change? The patch use loop induction variable analysis to find the register contain loop count. If the register is a read/write register, it would not identify as loop iv and SMS would skip the loop. Removing above condition and adding other code in loop-iv.c to identify this case. >> >> } >> >> +static rtx >> +get_rhs (rtx_insn *insn, rtx reg) > > > get_rhs might not be the most meaningful function name. We require > documentation before every function that says what it does, and what the > arguments mean. Please examine the surrounding code for examples. Ok, i have rename get_rhs to get_reg_calculate_expr and document before the function. >> >> > > This all looks a little odd; if you're looking for autoincs, why not just > scan the entire INSN for a MEM, rather than classify it as mem_write or > mem_read_insn? Yes, you're right. I have remove mem_read/write_insn_p and scan the INSN directly. > > > > Bernd Please reference the new submission by following link. https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00375.html Thanks, Shiva
