On Thu, Jul 11, 2019 at 02:52:03PM -0500, Segher Boessenkool wrote:
> On Wed, Jul 10, 2019 at 05:17:48PM -0400, Michael Meissner wrote:
> > +extern alias_set_type get_pc_relative_alias_set (void);
>
> I'd just call it "pcrel", not "pc_relative", just like everywhere else?
>
> > @@ -7785,7 +7787,7 @@ bool
> > toc_relative_expr_p (const_rtx op, bool strict, const_rtx *tocrel_base_ret,
> > const_rtx *tocrel_offset_ret)
> > {
> > - if (!TARGET_TOC)
> > + if (!TARGET_TOC || TARGET_PCREL)
>
> Maybe (TARGET_TOC && !TARGET_PCREL) should get a name of its own? Or
> maybe TARGET_TOC should not be enabled when TARGET_PCREL is? It doesn't
> make much sense at all to have both enabled, does it?
Yeah, in theory TARGET_TOC should not be set if TARGET_PCREL is set, but when I
last tried it, there were some failures. I can make a macro for the two
together.
> > + /* If this is a SYMBOL_REF that we refer to via pc-relative
> > addressing,
> > + we don't have to do any special for it. */
> > + else if (TARGET_PCREL && SYMBOL_REF_P (operands[1])
> > + && TARGET_CMODEL == CMODEL_MEDIUM
> > + && SYMBOL_REF_LOCAL_P (operands[1]))
>
> else if (TARGET_PCREL
> && TARGET_CMODEL == CMODEL_MEDIUM
> && SYMBOL_REF_P (operands[1])
> && SYMBOL_REF_LOCAL_P (operands[1]))
>
> > @@ -9928,6 +9938,11 @@ rs6000_emit_move (rtx dest, rtx source,
> > operands[1] = gen_const_mem (mode, tocref);
> > set_mem_alias_set (operands[1], get_TOC_alias_set ());
> > }
> > +
> > + else if (TARGET_PCREL && SYMBOL_REF_P (XEXP (operands[1], 0))
> > + && TARGET_CMODEL == CMODEL_MEDIUM
> > + && SYMBOL_REF_LOCAL_P (XEXP (operands[1], 0)))
> > + set_mem_alias_set (operands[1], get_pc_relative_alias_set ());
>
> Similar.
I had it in a function that did the checking, but after eliminating the
constant pool check that TOC needs, it was just doing the medium code model and
symbol_ref local checks, so I eliminated the function.
> > alias_set_type
> > get_TOC_alias_set (void)
> > {
> > - if (set == -1)
> > - set = new_alias_set ();
> > - return set;
> > + if (TOC_alias_set == -1)
> > + TOC_alias_set = new_alias_set ();
> > + return TOC_alias_set;
> > +}
>
> It would be nice if you could initialise the alias sets some other way.
> Not new code, but you duplicate it ;-)
For the pc-relative case, I'm not sure we need the alias set. In the TOC case,
we do a force_const_mem which returns a SYMBOL_REF pointing to the data in the
constant pool. Then we call create_TOC_reference which rewrites the memory,
and then it calls gen_const_mem which notes that memory in unchanging.
But for pc-relative, we just use output of force_const_mem. Now
force_const_mem does not seem to set the alias set (but it will have the
constant pool bit set).
> > +alias_set_type
> > +get_pc_relative_alias_set (void)
> > +{
> > + if (pc_relative_alias_set == -1)
> > + pc_relative_alias_set = new_alias_set ();
> > + return pc_relative_alias_set;
> > }
>
> Rest looks fine.
>
>
> Segher
>
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
