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