Re: [PATCH 2/2] gcc/genrecog: Don't warn for missing mode on special predicates

2016-07-13 Thread Jeff Law 

On 07/06/2016 01:42 PM, Andrew Burgess wrote:

* Richard Sandiford  [2016-07-04 09:47:20 +0100]:

Thanks for removing the duplicated error check for unknown predicates.
I think that error gets reported later though, so we should check for
null here:

  return pred && pred->special;

OK with that change, thanks.


Richard,

Thanks for the continued reviews.  I don't have GCC write access, so I
wonder if you would be willing to commit this patch for me please.

There's an updated version below that includes the latest change you
suggested.

Many thanks,
Andrew

---

[PATCH] gcc/genrecog: Don't warn for missing mode on special predicates

In md.texi it says:

  Predicates written with @code{define_special_predicate} do not get any
  automatic mode checks, and are treated as having special mode handling
  by @command{genrecog}.

In genrecog, when validating a SET pattern, there is already a special
case for 'address_operand' which is a special predicate, however,
other special predicates fall through to the code which checks for
incorrect use of VOIDmode.

This commit adds a new function for detecting special predicates, and
then generalises the check in validate_pattern so that mode checking
is skipped for all special predicates.

gcc/ChangeLog:

* genrecog.c (special_predicate_operand_p): New function.
(predicate_name): Move function.
(validate_pattern): Don't warn about missing mode for all
define_special_predicate predicates.

Committed.  THanks,

jeff

Re: [PATCH 2/2] gcc/genrecog: Don't warn for missing mode on special predicates

2016-07-06 Thread Andrew Burgess 
* Richard Sandiford  [2016-07-04 09:47:20 +0100]:

> Andrew Burgess  writes:
> > +/* Return true if OPERAND is a MATCH_OPERAND using a special predicate
> > +   function.  */
> > +
> > +static bool
> > +special_predicate_operand_p (rtx operand)
> > +{
> > +  if (GET_CODE (operand) == MATCH_OPERAND)
> > +{
> > +  const char *pred_name = predicate_name (operand);
> > +  if (pred_name[0] != 0)
> > +   {
> > + const struct pred_data *pred;
> > +
> > + pred = lookup_predicate (pred_name);
> > + return pred->special;
> 
> Thanks for removing the duplicated error check for unknown predicates.
> I think that error gets reported later though, so we should check for
> null here:
> 
>   return pred && pred->special;
> 
> OK with that change, thanks.

Richard,

Thanks for the continued reviews.  I don't have GCC write access, so I
wonder if you would be willing to commit this patch for me please.

There's an updated version below that includes the latest change you
suggested.

Many thanks,
Andrew

---

[PATCH] gcc/genrecog: Don't warn for missing mode on special predicates

In md.texi it says:

  Predicates written with @code{define_special_predicate} do not get any
  automatic mode checks, and are treated as having special mode handling
  by @command{genrecog}.

In genrecog, when validating a SET pattern, there is already a special
case for 'address_operand' which is a special predicate, however,
other special predicates fall through to the code which checks for
incorrect use of VOIDmode.

This commit adds a new function for detecting special predicates, and
then generalises the check in validate_pattern so that mode checking
is skipped for all special predicates.

gcc/ChangeLog:

* genrecog.c (special_predicate_operand_p): New function.
(predicate_name): Move function.
(validate_pattern): Don't warn about missing mode for all
define_special_predicate predicates.
---
 gcc/ChangeLog  |  7 +++
 gcc/genrecog.c | 50 +++---
 2 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/gcc/genrecog.c b/gcc/genrecog.c
index a9f5a4a..056798c 100644
--- a/gcc/genrecog.c
+++ b/gcc/genrecog.c
@@ -463,6 +463,38 @@ constraints_supported_in_insn_p (rtx insn)
   || GET_CODE (insn) == DEFINE_PEEPHOLE2);
 }
 
+/* Return the name of the predicate matched by MATCH_RTX.  */
+
+static const char *
+predicate_name (rtx match_rtx)
+{
+  if (GET_CODE (match_rtx) == MATCH_SCRATCH)
+return "scratch_operand";
+  else
+return XSTR (match_rtx, 1);
+}
+
+/* Return true if OPERAND is a MATCH_OPERAND using a special predicate
+   function.  */
+
+static bool
+special_predicate_operand_p (rtx operand)
+{
+  if (GET_CODE (operand) == MATCH_OPERAND)
+{
+  const char *pred_name = predicate_name (operand);
+  if (pred_name[0] != 0)
+   {
+ const struct pred_data *pred;
+
+ pred = lookup_predicate (pred_name);
+ return pred != NULL && pred->special;
+   }
+}
+
+  return false;
+}
+
 /* Check for various errors in PATTERN, which is part of INFO.
SET is nonnull for a destination, and is the complete set pattern.
SET_CODE is '=' for normal sets, and '+' within a context that
@@ -651,10 +683,9 @@ validate_pattern (rtx pattern, md_rtx_info *info, rtx set, 
int set_code)
dmode = GET_MODE (dest);
smode = GET_MODE (src);
 
-   /* The mode of an ADDRESS_OPERAND is the mode of the memory
-  reference, not the mode of the address.  */
-   if (GET_CODE (src) == MATCH_OPERAND
-   && ! strcmp (XSTR (src, 1), "address_operand"))
+   /* Mode checking is not performed for special predicates.  */
+   if (special_predicate_operand_p (src)
+   || special_predicate_operand_p (dest))
  ;
 
 /* The operands of a SET must have the same mode unless one
@@ -3788,17 +3819,6 @@ operator < (const pattern_pos &e1, const pattern_pos &e2)
   return diff < 0;
 }
 
-/* Return the name of the predicate matched by MATCH_RTX.  */
-
-static const char *
-predicate_name (rtx match_rtx)
-{
-  if (GET_CODE (match_rtx) == MATCH_SCRATCH)
-return "scratch_operand";
-  else
-return XSTR (match_rtx, 1);
-}
-
 /* Add new decisions to S that check whether the rtx at position POS
matches PATTERN.  Return the state that is reached in that case.
TOP_PATTERN is the overall pattern, as passed to match_pattern_1.  */
-- 
2.5.1

Re: [PATCH 2/2] gcc/genrecog: Don't warn for missing mode on special predicates

2016-07-04 Thread Richard Sandiford 
Andrew Burgess  writes:
> +/* Return true if OPERAND is a MATCH_OPERAND using a special predicate
> +   function.  */
> +
> +static bool
> +special_predicate_operand_p (rtx operand)
> +{
> +  if (GET_CODE (operand) == MATCH_OPERAND)
> +{
> +  const char *pred_name = predicate_name (operand);
> +  if (pred_name[0] != 0)
> + {
> +   const struct pred_data *pred;
> +
> +   pred = lookup_predicate (pred_name);
> +   return pred->special;

Thanks for removing the duplicated error check for unknown predicates.
I think that error gets reported later though, so we should check for
null here:

  return pred && pred->special;

OK with that change, thanks.

Richard

Re: [PATCH 2/2] gcc/genrecog: Don't warn for missing mode on special predicates

2016-06-30 Thread Andrew Burgess 
* Richard Sandiford  [2016-06-15 19:07:56 +0100]:

> Andrew Burgess  writes:
> > In md.texi it says:
> >
> >   Predicates written with @code{define_special_predicate} do not get any
> >   automatic mode checks, and are treated as having special mode handling
> >   by @command{genrecog}.
> >
> > However, in genrecog, when validating a SET pattern, if either the
> > source or destination is missing a mode then a warning is given, even if
> > there's a predicate defined with define_special_predicate.
> >
> > This commit silences the warning for special predicates.
> >
> > gcc/ChangeLog:
> >
> > * genrecog.c (validate_pattern): Don't warn about missing mode for
> > define_special_predicate predicates.
> > Acked-by: Andrew Burgess 
> > ---
> >  gcc/ChangeLog  |  5 +
> >  gcc/genrecog.c | 22 +++---
> >  2 files changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/genrecog.c b/gcc/genrecog.c
> > index a9f5a4a..7596552 100644
> > --- a/gcc/genrecog.c
> > +++ b/gcc/genrecog.c
> > @@ -674,9 +674,25 @@ validate_pattern (rtx pattern, md_rtx_info *info, rtx 
> > set, int set_code)
> >  && !CONST_WIDE_INT_P (src)
> >  && GET_CODE (src) != CALL)
> >   {
> > -   const char *which;
> > -   which = (dmode == VOIDmode ? "destination" : "source");
> > -   message_at (info->loc, "warning: %s missing a mode?", which);
> > +   const char *which_msg;
> > +   rtx which;
> > +   const char *pred_name;
> > +   const struct pred_data *pred;
> > +
> > +   which_msg = (dmode == VOIDmode ? "destination" : "source");
> > +   which = (dmode == VOIDmode ? dest : src);
> > +   pred_name = XSTR (which, 1);
> > +   if (pred_name[0] != 0)
> > + {
> > +   pred = lookup_predicate (pred_name);
> > +   if (!pred)
> > + error_at (info->loc, "unknown predicate '%s'", pred_name);
> > + }
> > +   else
> > + pred = 0;
> > +   if (!pred || !pred->special)
> > + message_at (info->loc, "warning: %s missing a mode?",
> > + which_msg);
> 
> There's no guarantee at this point that "which" is a match_operand.
> Also, I think the earlier:
> 
> /* The operands of a SET must have the same mode unless one
>  is VOIDmode.  */
> else if (dmode != VOIDmode && smode != VOIDmode && dmode != smode)
> error_at (info->loc, "mode mismatch in set: %smode vs %smode",
>   GET_MODE_NAME (dmode), GET_MODE_NAME (smode));
> 
> should be skipped for special predicates too.
> 
> How about generalising:
> 
>   /* The mode of an ADDRESS_OPERAND is the mode of the memory
>  reference, not the mode of the address.  */
>   if (GET_CODE (src) == MATCH_OPERAND
>   && ! strcmp (XSTR (src, 1), "address_operand"))
> ;
> 
> to:
> 
>   if (special_predicate_operand_p (src)
>   || special_predicate_operand_p (dest))
> ;
> 
> with a new special_predicate_operand_p helper?  I don't think we should
> duplicate the "unknown predicate" error here; the helper can just return
> false for unknown predicates.

Thanks for taking the time to review and provide feedback.  Sorry it
has take a while to get around to this patch again.

I've updated the patch inline with your feedback.  How's this?

Thanks,
Andrew

---

gcc/genrecog: Don't warn for missing mode on special predicates

In md.texi it says:

  Predicates written with @code{define_special_predicate} do not get any
  automatic mode checks, and are treated as having special mode handling
  by @command{genrecog}.

In genrecog, when validating a SET pattern, there is already a special
case for 'address_operand' which is a special predicate, however,
other special predicates fall through to the code which checks for
incorrect use of VOIDmode.

This commit adds a new function for detecting special predicates, and
then generalises the check in validate_pattern so that mode checking
is skipped for all special predicates.

gcc/ChangeLog:

* genrecog.c (special_predicate_operand_p): New function.
(predicate_name): Move function.
(validate_pattern): Don't warn about missing mode for all
define_special_predicate predicates.
---
 gcc/ChangeLog  |  7 +++
 gcc/genrecog.c | 50 +++---
 2 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/gcc/genrecog.c b/gcc/genrecog.c
index a9f5a4a..7c56225 100644
--- a/gcc/genrecog.c
+++ b/gcc/genrecog.c
@@ -463,6 +463,38 @@ constraints_supported_in_insn_p (rtx insn)
   || GET_CODE (insn) == DEFINE_PEEPHOLE2);
 }
 
+/* Return the name of the predicate matched by MATCH_RTX.  */
+
+static const char *
+predicate_name (rtx match_rtx)
+{
+  if (GET_CODE (match_rtx) == MATCH_SCRATCH)
+return "scratch_operand";
+  else
+return XSTR (match_rtx, 1);
+}
+
+/* Return true if OPERAND is a MATCH_OPERAND using a special predicate
+   funct

Re: [PATCH 2/2] gcc/genrecog: Don't warn for missing mode on special predicates

2016-06-15 Thread Richard Sandiford 
Andrew Burgess  writes:
> In md.texi it says:
>
>   Predicates written with @code{define_special_predicate} do not get any
>   automatic mode checks, and are treated as having special mode handling
>   by @command{genrecog}.
>
> However, in genrecog, when validating a SET pattern, if either the
> source or destination is missing a mode then a warning is given, even if
> there's a predicate defined with define_special_predicate.
>
> This commit silences the warning for special predicates.
>
> gcc/ChangeLog:
>
>   * genrecog.c (validate_pattern): Don't warn about missing mode for
>   define_special_predicate predicates.
> Acked-by: Andrew Burgess 
> ---
>  gcc/ChangeLog  |  5 +
>  gcc/genrecog.c | 22 +++---
>  2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/genrecog.c b/gcc/genrecog.c
> index a9f5a4a..7596552 100644
> --- a/gcc/genrecog.c
> +++ b/gcc/genrecog.c
> @@ -674,9 +674,25 @@ validate_pattern (rtx pattern, md_rtx_info *info, rtx 
> set, int set_code)
>&& !CONST_WIDE_INT_P (src)
>&& GET_CODE (src) != CALL)
> {
> - const char *which;
> - which = (dmode == VOIDmode ? "destination" : "source");
> - message_at (info->loc, "warning: %s missing a mode?", which);
> + const char *which_msg;
> + rtx which;
> + const char *pred_name;
> + const struct pred_data *pred;
> +
> + which_msg = (dmode == VOIDmode ? "destination" : "source");
> + which = (dmode == VOIDmode ? dest : src);
> + pred_name = XSTR (which, 1);
> + if (pred_name[0] != 0)
> +   {
> + pred = lookup_predicate (pred_name);
> + if (!pred)
> +   error_at (info->loc, "unknown predicate '%s'", pred_name);
> +   }
> + else
> +   pred = 0;
> + if (!pred || !pred->special)
> +   message_at (info->loc, "warning: %s missing a mode?",
> +   which_msg);

There's no guarantee at this point that "which" is a match_operand.
Also, I think the earlier:

/* The operands of a SET must have the same mode unless one
   is VOIDmode.  */
else if (dmode != VOIDmode && smode != VOIDmode && dmode != smode)
  error_at (info->loc, "mode mismatch in set: %smode vs %smode",
GET_MODE_NAME (dmode), GET_MODE_NAME (smode));

should be skipped for special predicates too.

How about generalising:

/* The mode of an ADDRESS_OPERAND is the mode of the memory
   reference, not the mode of the address.  */
if (GET_CODE (src) == MATCH_OPERAND
&& ! strcmp (XSTR (src, 1), "address_operand"))
  ;

to:

if (special_predicate_operand_p (src)
|| special_predicate_operand_p (dest))
  ;

with a new special_predicate_operand_p helper?  I don't think we should
duplicate the "unknown predicate" error here; the helper can just return
false for unknown predicates.

Thanks,
Richard

[PATCH 2/2] gcc/genrecog: Don't warn for missing mode on special predicates

2016-06-14 Thread Andrew Burgess 
In md.texi it says:

  Predicates written with @code{define_special_predicate} do not get any
  automatic mode checks, and are treated as having special mode handling
  by @command{genrecog}.

However, in genrecog, when validating a SET pattern, if either the
source or destination is missing a mode then a warning is given, even if
there's a predicate defined with define_special_predicate.

This commit silences the warning for special predicates.

gcc/ChangeLog:

* genrecog.c (validate_pattern): Don't warn about missing mode for
define_special_predicate predicates.
Acked-by: Andrew Burgess 
---
 gcc/ChangeLog  |  5 +
 gcc/genrecog.c | 22 +++---
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/gcc/genrecog.c b/gcc/genrecog.c
index a9f5a4a..7596552 100644
--- a/gcc/genrecog.c
+++ b/gcc/genrecog.c
@@ -674,9 +674,25 @@ validate_pattern (rtx pattern, md_rtx_info *info, rtx set, 
int set_code)
 && !CONST_WIDE_INT_P (src)
 && GET_CODE (src) != CALL)
  {
-   const char *which;
-   which = (dmode == VOIDmode ? "destination" : "source");
-   message_at (info->loc, "warning: %s missing a mode?", which);
+   const char *which_msg;
+   rtx which;
+   const char *pred_name;
+   const struct pred_data *pred;
+
+   which_msg = (dmode == VOIDmode ? "destination" : "source");
+   which = (dmode == VOIDmode ? dest : src);
+   pred_name = XSTR (which, 1);
+   if (pred_name[0] != 0)
+ {
+   pred = lookup_predicate (pred_name);
+   if (!pred)
+ error_at (info->loc, "unknown predicate '%s'", pred_name);
+ }
+   else
+ pred = 0;
+   if (!pred || !pred->special)
+ message_at (info->loc, "warning: %s missing a mode?",
+ which_msg);
  }
 
if (dest != SET_DEST (pattern))
-- 
2.6.4