On Sun, May 10, 2015 at 07:02:33PM +0100, Maciej W. Rozycki wrote: > > -(define_insn "rlwinm" > > +(define_insn "*ashlsi3_imm_mask" > > [(set (match_operand:SI 0 "gpc_reg_operand" "=r") > > (and:SI (ashift:SI (match_operand:SI 1 "gpc_reg_operand" "r") > > (match_operand:SI 2 "const_int_operand" "i")) > > This clearly renames rather than removing the `rlwinm' pattern, please > correctly reflect that in ChangeLog. Some other, unnamed patterns are > given names rather than deleted as well, just as you've noted at the top. > And none of the other changes are mentioned in your ChangeLog entry.
The changelog says it deletes the patterns with the old names. Which it does. And it adds the ones with the new names. Which it does. No one said changelogs are useful ;-) > Would you be able to split this change up further by any chance? I could, but it would be a lot of extra work. First, I haven't done those steps in order (they aren't "steps" at all): I take one pattern, and fix it up, then the next, etc. But much worse: if you do tiny pattern changes like you suggest, I can guarantee you the patches _will_ mis-apply. All of the time :-( [ Sometimes two patterns even have the same name, and sometimes even 100% identical contents. Exciting! ] > Perhaps into the very steps you listed at the top so that each individual > change addresses a single issue only. That would avoid problems with > ChangeLog and make the review easier. This isn't the first patch like this, I've cleaned up most other PowerPC integer patterns already. It's enough work already. Often the changes cannot be separated, and even if they can you then need them in the fixed order you made for them, etc. For this small patch I could probably do a better changelog though. Segher
