This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH 6/6] rs6000: Clean up the various rlwinm patterns
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: "Maciej W. Rozycki" <macro at linux-mips dot org>
- Cc: gcc-patches at gcc dot gnu dot org, dje dot gcc at gmail dot com
- Date: Sun, 10 May 2015 14:44:53 -0500
- Subject: Re: [PATCH 6/6] rs6000: Clean up the various rlwinm patterns
- Authentication-results: sourceware.org; auth=none
- References: <cover dot 1431268134 dot git dot segher at kernel dot crashing dot org> <31a8639608e417c3b2d23cf3cee84fa7f5720aed dot 1431268135 dot git dot segher at kernel dot crashing dot org> <alpine dot LFD dot 2 dot 11 dot 1505101845180 dot 1538 at eddie dot linux-mips dot org>
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.