On Wed, 2017-06-28 at 18:19 -0500, Segher Boessenkool wrote:
> On Wed, Jun 28, 2017 at 03:21:49PM -0500, Aaron Sawdey wrote:
> > -toc_relative_expr_p (const_rtx op, bool strict)
> > +toc_relative_expr_p (const_rtx op, bool strict, const_rtx
> > *tocrel_base_ret,
> > +                const_rtx *tocrel_offset_ret)
> >  {
> >    if (!TARGET_TOC)
> >      return false;
> > -  tocrel_base = op;
> > -  tocrel_offset = const0_rtx;
> > +  const_rtx tocrel_base = op;
> > +  const_rtx tocrel_offset = const0_rtx;
> > +
> >    if (GET_CODE (op) == PLUS && add_cint_operand (XEXP (op, 1),
> > GET_MODE (op)))
> >      {
> >        tocrel_base = XEXP (op, 0);
> > -      tocrel_offset = XEXP (op, 1);
> > +      if (tocrel_offset_ret)
> > +   tocrel_offset = XEXP (op, 1);
> 
> Lose the "if"?  Or do you get a compiler warning then?

I was just trying to avoid unnecessary work in the case where the
pointer is NULL. In that case tocrel_offset isn't actually used for
anything. Probably I should just let the compiler figure that one out,
I will delete the if for clarity.

> > @@ -8674,7 +8686,8 @@
> >  legitimate_constant_pool_address_p (const_rtx x, machine_mode
> > mode,
> >                                 bool strict)
> >  {
> > -  return (toc_relative_expr_p (x, strict)
> > +  const_rtx tocrel_base, tocrel_offset;
> > +  return (toc_relative_expr_p (x, strict, &tocrel_base,
> > &tocrel_offset)
> >       && (TARGET_CMODEL != CMODEL_MEDIUM
> >           || constant_pool_expr_p (XVECEXP (tocrel_base, 0,
> > 0))
> >           || mode == QImode
> 
> Use NULL for the args here, instead?

The diff didn't include all the context. Both tocrel_base and
tocrel_offset are used in the function:

bool
legitimate_constant_pool_address_p (const_rtx x, machine_mode mode,
                                    bool strict)
{
  const_rtx tocrel_base, tocrel_offset;
  return (toc_relative_expr_p (x, strict, &tocrel_base, &tocrel_offset)
          && (TARGET_CMODEL != CMODEL_MEDIUM
              || constant_pool_expr_p (XVECEXP (tocrel_base, 0, 0))
              || mode == QImode
              || offsettable_ok_by_alignment (XVECEXP (tocrel_base, 0, 0),
                                              INTVAL (tocrel_offset), mode)));
}


> 
> The patch is okay for trunk with those things taken care of.  Thanks,
> 
> 
> Segher
> 
-- 
Aaron Sawdey, Ph.D.  acsaw...@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain

Reply via email to