Re: [PATCH v3] Modify combine pattern by a pseudo AND with its nonzero bits [PR93453]

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Thu, Aug 11, 2022 at 10:11:45AM +0800, HAO CHEN GUI wrote:
> On 11/8/2022 上午 1:38, Segher Boessenkool wrote:
> > On Fri, Jul 22, 2022 at 03:07:55PM +0800, HAO CHEN GUI wrote:
> >>   This patch creates a new function - change_pseudo_and_mask. If recog fails,
> >> the function converts a single pseudo to the pseudo AND with a mask if the
> >> outer operator is IOR/XOR/PLUS and inner operator is ASHIFT or AND. The
> >> conversion helps pattern to match rotate and mask insn on some targets.
> > 
> > The name isn't so clear.  It isn't changing a mask, to start with.
> How about setting function name to change_pseudo? It converts a pseudo to
> the pseudo AND with a mask in a particular situation.

Change pseudo to what?  It remains a pseudo, the same pseudo even.

"add_mask_to_pseudo"...  Well, that says "add", which is like "plus",
not a great name either -- but something in that direction?

> >> +/* When the outer code of set_src is IOR/XOR/PLUS and the inner code is
> >> +   ASHIFT/AND,
> > 
> > "When the outercode of the SET_SRC of PAT is ..."
> Yeah, I will change it.

"outer code" of course, sorry, I typed it wrong :-/

> >> convert a pseudo to pseudo AND with a mask if its nonzero_bits
> >> +   is less than its mode mask.  The nonzero_bits in later passes is not a
> >> +   superset of what is known in combine pass.  So an insn with nonzero_bits
> >> +   can't be recoged later.  */
> > 
> > Can this not be done with a splitter in the machine description?
> > 
> Sorry, I don't quite understand it. Do you mean if the conversion can be done in
> split pass?

In any splitter, also during combine for example.  Is this common enough
to warrant special code in combine to handle all tens of cases, or is it
better handled in separate patterns?

> If a pseudo has DImode and stem from a char, we get nonzero_bits as 0xff in combine
> pass. But in split pass, it's nonzero_bits is 0xffffffffffffffff. So the conversion
> can only be done in combine pass.

Yes, but combine will use splitters as well.

You can use nonzero_bits in the split condition (the second condition in
a define_split, or the sole condition in a define_split) just fine, as
long as the replacement RTL does not rely on the nonzero_bits itself.
You cannot use it in the insn condition (the first condition in a
define_insn_and_split, or the one condition in a define_insn) because
that RTL will survive past combine, and then nonzero_bits can have bits
cleared that were set before (that were determined to be always zero
during combine, but that knowledge is gone later).

(Insns can also be split in thread_pro_and_epilogue, during reorg, and
in final of course).


Segher