Re: [RS6000] rs6000_rtx_costs cost IOR

[Alan Modra <amodra@gmail.com>]

On Wed, Sep 16, 2020 at 07:02:06PM -0500, Segher Boessenkool wrote:
> Hi!
> 
> On Tue, Sep 15, 2020 at 10:49:44AM +0930, Alan Modra wrote:
> > 	* config/rs6000/rs6000.c (rs6000_rtx_costs): Cost IOR.
> 
> >      case IOR:
> > -      /* FIXME */
> >        *total = COSTS_N_INSNS (1);
> > -      return true;
> 
> Hey this was okay for over five years :-)
> 
> > +      left = XEXP (x, 0);
> > +      if (GET_CODE (left) == AND
> > +	  && CONST_INT_P (XEXP (left, 1)))
> 
> Add a comment that this is the integer insert insns?
> 
> > +	      // rotlsi3_insert_5
> 
> But use /* comments */.
> 
> > +		  /* Test both regs even though the one in the mask is
> > +		     constrained to be equal to the output.  Increasing
> > +		     cost may well result in rejecting an invalid insn
> > +		     earlier.  */
> 
> Is that ever actually useful?

Possibly not in this particular case, but I did see cases where
invalid insns were rejected early by costing non-reg sub-expressions.

Beside that, the default position on rtx_costs paths that return true
should be to cost any sub-expressions unless you know for sure they
are zero cost.  And yes, we fail to do that for some cases,
eg. mul_highpart.

> So this new block is pretty huge.  Can it easily be factored to a
> separate function?  Just the insert insns part, not all IOR.

Done in my local tree.

> Okay for trunk with the comments changed to the correct syntax, and
> factoring masked insert out to a separate function pre-approved if you
> want to do that.  Thanks!

I'll hold off committing until the whole rtx_costs patch series is
reviewed (not counting the rotate_and_mask_constant patch).

-- 
Alan Modra
Australia Development Lab, IBM