On Tue, Feb 27, 2024 at 04:50:02PM -0600, Peter Bergner wrote:
> On 2/27/24 6:40 AM, Segher Boessenkool wrote:
> > On Tue, Feb 27, 2024 at 02:02:38AM +0530, jeevitha wrote:
> > input_operand allows a lot of things that splat_input_operand does not,
> > not just immediate operands.  NAK.
> > 
> > (For example, *all* memory is okay for input_operand, always).
> > 
> > I'm not saying we do not want to restrict these things, but a commit
> > that doesn't discuss this at all is not okay.  Sorry.
> 
> So it seems you're not NAKing the use of splat_input_operand, but
> just that it needs more explanation in the git log entry, correct?

I NAK the patch.  _Of course_ there needs to be *something* done, there
is a bug after all, it needs to be fixed.

But no, there are big questions about if splat_input_operand is correct
as well.  This needs to be justified in the patch submission.

> Yes, input_operand accepts a lot more things than splat_input_operand
> does, but the multiple define_insns this define_expand feeds, uses
> gpc_reg_operand, memory_operand and splat_input_operand for their
> operands[1] operand (splat_input_operand accepts reg and mem too),
> so it seems to match better what the patterns will be accepting and
> I always thought that using predicates that more accurately reflect
> what the define_insns expect/accept lead to better code gen.

Still, it needs explanation why we allowed it before, but that was a
mistake, or for some reason we do not need it.

Sell your patch!  :-)

> Mike, was it just an oversight to not use splat_input_operand for the
> vsx_splat_<mode> expander or was input_operand a conscious decision?
> 
> If input_operand was used purposely, then we can just fall back to
> the s/op1/operands[1]/ change which we already know fixes the bug.

input_operand allows all inputs for mov insns.  It isn't suitable
for any other instructions.


Segher

Reply via email to