On Wed, Jun 28, 2017 at 03:21:49PM -0500, Aaron Sawdey wrote:
> > It is probably nicer to have a separate function for
> > toc_relative_expr_p
> > and one to pull the base/offset out. And maybe don't keep it cached
> > for
> > the output function either? It has all info it needs, right, the
> > full
> > address RTX? I don't think it is measurably slower to pull the
> > address
> > apart an extra time?
>
> I think it doesn't make a lot of sense to have two functions as you
> have to do nearly all the work just to get the true/false return value,
> you have to completely compute tocrel_base.
Right, but how much work is that? And it will get rid of all the cached
stuff, simplifying things quite a bit. None of this is new in your
patch of course.
> /* Return true if OP is a toc pointer relative address (the output
> of create_TOC_reference). If STRICT, do not match non-split
> - -mcmodel=large/medium toc pointer relative addresses. */
> + -mcmodel=large/medium toc pointer relative addresses. Place base
> + and offset pieces in TOCREL_BASE and TOCREL_OFFSET respectively. */
"If those are non-NULL"?
> -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?
> @@ -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 patch is okay for trunk with those things taken care of. Thanks,
Segher
