Re: [PATCH] V10 patch #4, Add new prefixed/non-prefixed memory constraints
Hi! On Tue, Dec 17, 2019 at 07:38:51PM -0500, Michael Meissner wrote: > On Tue, Dec 17, 2019 at 05:35:24PM -0600, Segher Boessenkool wrote: > > And what is with the INSN_FORM_PCREL_EXTERNAL? > > INSN_FORM_PCREL_EXTERNAL says that the operand is a reference to an external > symbol. It cannot appear in an actual memory insns in normal usage, but it > needs to be handled several places: Sure. Both prefixed_memory and non_prefixed_memory should test something like memory_operand, not just whether it is a MEM. But *both* of them, that's the point, and using some more generic hook. Segher
Re: [PATCH] V10 patch #4, Add new prefixed/non-prefixed memory constraints
On Tue, Dec 17, 2019 at 05:35:24PM -0600, Segher Boessenkool wrote:
> On Tue, Dec 17, 2019 at 05:29:44PM -0500, Michael Meissner wrote:
> > On Tue, Dec 17, 2019 at 11:15:29AM -0600, Segher Boessenkool wrote:
> > > > +;; Return true if the operand is a valid memory address that does not
> > > > use a
> > > > +;; prefixed address.
> > > > +(define_predicate "non_prefixed_memory"
> > > > + (match_code "mem")
> > > > +{
> > > > + enum insn_form iform
> > > > += address_to_insn_form (XEXP (op, 0), mode, NON_PREFIXED_DEFAULT);
> > > > +
> > > > + return (iform != INSN_FORM_BAD
> > > > + && iform != INSN_FORM_PREFIXED_NUMERIC
> > > > + && iform != INSN_FORM_PCREL_LOCAL
> > > > + && iform != INSN_FORM_PCREL_EXTERNAL);
> > > > +})
> > >
> > > Why can this not use just !address_is_prefixed? Why is an
> > > INSN_FORM_PCREL_EXTERNAL address neither prefixed nor non-prefixed? What
> > > does "BAD" mean, really? Should that ever happen, should that not ICE?
> >
> > You can't just invert !address_is_prefixed, because it would all things that
> > may not be valid memory addresses.
>
> Yes, so test that *explicitly*, in the "prefixed_memory" predicate as
> well please. Make the two predicates as much the same as possible.
>
> And what is with the INSN_FORM_PCREL_EXTERNAL?
INSN_FORM_PCREL_EXTERNAL says that the operand is a reference to an external
symbol. It cannot appear in an actual memory insns in normal usage, but it
needs to be handled several places:
1) pcrel_extern_addr needs to be able to load an external address into a GPR
register.
2) The prefixed insn attribute (and prefixed_paddi_p which it calls) needs to
recognize pcrel_extern_addr and note that it is prefixed.
3) The PCREL_OPT support will need to support it. If you do the PCREL_OPT
support via combine and flow control passes, you will need to be able to handle
external references as addresses.
The function address_is_prefixed, specifically does not return true for
external symbols, because you can't use them in a normal context.
In the context of the patch (vector extract), it needs to decide whether the
address is prefixed or not, in order to decide whether it needs a second base
register temporary.
--
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
Re: [PATCH] V10 patch #4, Add new prefixed/non-prefixed memory constraints
On Tue, Dec 17, 2019 at 05:29:44PM -0500, Michael Meissner wrote:
> On Tue, Dec 17, 2019 at 11:15:29AM -0600, Segher Boessenkool wrote:
> > > +;; Return true if the operand is a valid memory address that does not
> > > use a
> > > +;; prefixed address.
> > > +(define_predicate "non_prefixed_memory"
> > > + (match_code "mem")
> > > +{
> > > + enum insn_form iform
> > > += address_to_insn_form (XEXP (op, 0), mode, NON_PREFIXED_DEFAULT);
> > > +
> > > + return (iform != INSN_FORM_BAD
> > > + && iform != INSN_FORM_PREFIXED_NUMERIC
> > > + && iform != INSN_FORM_PCREL_LOCAL
> > > + && iform != INSN_FORM_PCREL_EXTERNAL);
> > > +})
> >
> > Why can this not use just !address_is_prefixed? Why is an
> > INSN_FORM_PCREL_EXTERNAL address neither prefixed nor non-prefixed? What
> > does "BAD" mean, really? Should that ever happen, should that not ICE?
>
> You can't just invert !address_is_prefixed, because it would all things that
> may not be valid memory addresses.
Yes, so test that *explicitly*, in the "prefixed_memory" predicate as
well please. Make the two predicates as much the same as possible.
And what is with the INSN_FORM_PCREL_EXTERNAL?
Segher
Re: [PATCH] V10 patch #4, Add new prefixed/non-prefixed memory constraints
On Tue, Dec 17, 2019 at 11:15:29AM -0600, Segher Boessenkool wrote:
> Hi!
>
> On Wed, Dec 11, 2019 at 07:29:05PM -0500, Michael Meissner wrote:
> > +(define_memory_constraint "em"
> > + "A memory operand that does not contain a prefixed address."
> > + (and (match_code "mem")
> > + (match_operand 0 "non_prefixed_memory")))
> > +
> > +(define_memory_constraint "ep"
> > + "A memory operand that does contains a prefixed address."
> > + (and (match_code "mem")
> > + (match_operand 0 "prefixed_memory")))
>
> "does contain". Or maybe just say "with a non-prefixed address" and
> "with a prefixed address"?
Ok.
> > +;; Return true if the operand is a valid memory address that does not use a
> > +;; prefixed address.
> > +(define_predicate "non_prefixed_memory"
> > + (match_code "mem")
> > +{
> > + enum insn_form iform
> > += address_to_insn_form (XEXP (op, 0), mode, NON_PREFIXED_DEFAULT);
> > +
> > + return (iform != INSN_FORM_BAD
> > + && iform != INSN_FORM_PREFIXED_NUMERIC
> > + && iform != INSN_FORM_PCREL_LOCAL
> > + && iform != INSN_FORM_PCREL_EXTERNAL);
> > +})
>
> Why can this not use just !address_is_prefixed? Why is an
> INSN_FORM_PCREL_EXTERNAL address neither prefixed nor non-prefixed? What
> does "BAD" mean, really? Should that ever happen, should that not ICE?
You can't just invert !address_is_prefixed, because it would all things that
may not be valid memory addresses.
So we could just do:
{
/* If the operand is not a valid memory operand even if it is not prefixed,
do not return true. */
if (!memory_operand (op, mode))
return false;
return !address_is_prefixed (XEXP (op, 0), mode, NON_PREFIXED_DEFAULT);
}
It is important that the predicate not return true if the operand is NOT a
valid memory address. If you allow non-valid memory addresses, the register
allocator will create things like:
(mem:MODE (plus:DI (reg:DI x)
(plus:DI (reg:DI y)
(const_int z
Or some such -- I forget the exact sequence it created. A later pass would
then choke with bad insn.
INSN_FORM_BAD just means that the operand is not valid as a memory address.
> It is very confusing if any valid memory is neither "prefixed_memory" nor
> "non_prefixed_memory"!
The point was to make sure the memory is valid. Once it is a valid memory
address, then just a simple !address_is_prefixed will work.
> > --- gcc/doc/md.texi (revision 279182)
> > +++ gcc/doc/md.texi (working copy)
> > @@ -3373,6 +3373,12 @@ asm ("st %1,%0" : "=m<>" (mem) : "r" (va
> >
> > is not.
> >
> > +@item em
> > +A memory operand that does not contain a prefixed address.
> > +
> > +@item ep
> > +A memory operand that does contains a prefixed address.
>
> Same comments as above.
Ok.
--
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
Re: [PATCH] V10 patch #4, Add new prefixed/non-prefixed memory constraints
Hi!
On Wed, Dec 11, 2019 at 07:29:05PM -0500, Michael Meissner wrote:
> +(define_memory_constraint "em"
> + "A memory operand that does not contain a prefixed address."
> + (and (match_code "mem")
> + (match_operand 0 "non_prefixed_memory")))
> +
> +(define_memory_constraint "ep"
> + "A memory operand that does contains a prefixed address."
> + (and (match_code "mem")
> + (match_operand 0 "prefixed_memory")))
"does contain". Or maybe just say "with a non-prefixed address" and
"with a prefixed address"?
> +;; Return true if the operand is a valid memory address that does not use a
> +;; prefixed address.
> +(define_predicate "non_prefixed_memory"
> + (match_code "mem")
> +{
> + enum insn_form iform
> += address_to_insn_form (XEXP (op, 0), mode, NON_PREFIXED_DEFAULT);
> +
> + return (iform != INSN_FORM_BAD
> + && iform != INSN_FORM_PREFIXED_NUMERIC
> + && iform != INSN_FORM_PCREL_LOCAL
> + && iform != INSN_FORM_PCREL_EXTERNAL);
> +})
Why can this not use just !address_is_prefixed? Why is an
INSN_FORM_PCREL_EXTERNAL address neither prefixed nor non-prefixed? What
does "BAD" mean, really? Should that ever happen, should that not ICE?
It is very confusing if any valid memory is neither "prefixed_memory" nor
"non_prefixed_memory"!
> --- gcc/doc/md.texi (revision 279182)
> +++ gcc/doc/md.texi (working copy)
> @@ -3373,6 +3373,12 @@ asm ("st %1,%0" : "=m<>" (mem) : "r" (va
>
> is not.
>
> +@item em
> +A memory operand that does not contain a prefixed address.
> +
> +@item ep
> +A memory operand that does contains a prefixed address.
Same comments as above.
Segher
