Patchwork PATCH: PR web/44775: union_match_dups failed to check NULL *ref

login
register
mail settings
Submitter H.J. Lu
Date July 1, 2010, 10:54 p.m.
Message ID <20100701225418.GA19170@intel.com>
Download mbox | patch
Permalink /patch/57605/
State New
Headers show

Comments

H.J. Lu - July 1, 2010, 10:54 p.m.
Hi,

This patch adds check for NULL *ref. OK to install?

Thanks.


H.J.
---
2010-07-01  H.J. Lu  <hongjiu.lu@intel.com>

	PR web/44775
	* web.c (union_match_dups): Check NULL *ref.
Bernd Schmidt - July 1, 2010, 11 p.m.
On 07/02/2010 12:54 AM, H.J. Lu wrote:
> 2010-07-01  H.J. Lu  <hongjiu.lu@intel.com>
> 
> 	PR web/44775
> 	* web.c (union_match_dups): Check NULL *ref.
> 
> diff --git a/gcc/web.c b/gcc/web.c
> index ff91733..4b2ce36 100644
> --- a/gcc/web.c
> +++ b/gcc/web.c
> @@ -123,7 +123,8 @@ union_match_dups (rtx insn, struct web_entry *def_entry,
>  	if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
>  	  break;
>  
> -      (*fun) (use_entry + DF_REF_ID (*dupref), entry + DF_REF_ID (*ref));
> +      if (*ref)
> +	(*fun) (use_entry + DF_REF_ID (*dupref), entry + DF_REF_ID (*ref));

This looks wrong.  If you have a match_dup, you must have a matching
operand.  Maybe a pattern isn't being generated correctly?


Bernd
H.J. Lu - July 1, 2010, 11:06 p.m.
On Thu, Jul 1, 2010 at 4:00 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 07/02/2010 12:54 AM, H.J. Lu wrote:
>> 2010-07-01  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>       PR web/44775
>>       * web.c (union_match_dups): Check NULL *ref.
>>
>> diff --git a/gcc/web.c b/gcc/web.c
>> index ff91733..4b2ce36 100644
>> --- a/gcc/web.c
>> +++ b/gcc/web.c
>> @@ -123,7 +123,8 @@ union_match_dups (rtx insn, struct web_entry *def_entry,
>>       if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
>>         break;
>>
>> -      (*fun) (use_entry + DF_REF_ID (*dupref), entry + DF_REF_ID (*ref));
>> +      if (*ref)
>> +     (*fun) (use_entry + DF_REF_ID (*dupref), entry + DF_REF_ID (*ref));
>
> This looks wrong.  If you have a match_dup, you must have a matching
> operand.  Maybe a pattern isn't being generated correctly?
>

Pattern is

(define_insn "divmodhiqi3"
  [(set (match_operand:HI 0 "register_operand" "=a")
        (ior:HI
          (ashift:HI
            (zero_extend:HI
              (mod:QI (subreg:QI
                        (match_operand:HI 1 "register_operand" "0") 0)
                      (match_operand:QI 2 "nonimmediate_operand" "qm")))
            (const_int 8))
          (zero_extend:HI
            (div:QI (subreg:QI (match_dup 1) 0) (match_dup 2)))))
   (use (match_dup 1))
   (clobber (reg:CC FLAGS_REG))]
  "TARGET_QIMODE_MATH"
  "idiv{b}\t%2"
  [(set_attr "type" "idiv")
   (set_attr "mode" "QI")])

I added "(use (match_dup 1))" since I am not sure if upper 8bits
in operand 1 will be touched after the whole 16bit is loaded
and subreg:QI is used on operand 1.

INSN is

(insn 216 215 217 40
/export/gnu/import/git/gcc/gcc/testsuite/gcc.c-torture/execute/arith-rand.c:101
(parallel [
            (set (reg:HI 854)
                (ior:HI (ashift:HI (zero_extend:HI (umod:QI (subreg:QI (reg:HI
855 [ xx ]) 0)
                                (reg/v:QI 114 [ yy ])))
                        (const_int 8 [0x8]))
                    (zero_extend:HI (udiv:QI (subreg:QI (reg:HI 855 [ xx ]) 0)
                            (reg/v:QI 114 [ yy ])))))
            (use (reg:HI 855 [ xx ]))
            (clobber (reg:CC 17 flags))
        ]) 350 {udivmodhiqi3} (expr_list:REG_DEAD (reg:HI 855 [ xx ])
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))

Thanks.
Bernd Schmidt - July 1, 2010, 11:18 p.m.
On 07/02/2010 01:06 AM, H.J. Lu wrote:
> Pattern is
> 
> (define_insn "divmodhiqi3"
>   [(set (match_operand:HI 0 "register_operand" "=a")
>         (ior:HI
>           (ashift:HI
>             (zero_extend:HI
>               (mod:QI (subreg:QI
>                         (match_operand:HI 1 "register_operand" "0") 0)
>                       (match_operand:QI 2 "nonimmediate_operand" "qm")))
>             (const_int 8))
>           (zero_extend:HI
>             (div:QI (subreg:QI (match_dup 1) 0) (match_dup 2)))))
>    (use (match_dup 1))
>    (clobber (reg:CC FLAGS_REG))]
>   "TARGET_QIMODE_MATH"
>   "idiv{b}\t%2"
>   [(set_attr "type" "idiv")
>    (set_attr "mode" "QI")])
> 
> I added "(use (match_dup 1))" since I am not sure if upper 8bits
> in operand 1 will be touched after the whole 16bit is loaded
> and subreg:QI is used on operand 1.

Using subreg like that is pretty ugly, but I'm not sure whether that's
actually the problem.  I think you want a QImode match_operand there.
Not sure what the point of this pattern (or the HImode operands) is, it
looks like it wants to be a standard name, but as far as I can tell from
the documentation, divmod only takes one mode.

If you want to represent a HImode division producing a QImode result,
that's probably

  (truncate:QI (div:HI (match_operand:HI 1)
                       (sign_extend:HI (match_operand:QI 2))))

and now that I've checked the manual, that even appears as an example.

The use seems unnecessary.

Try to fix the pattern along these lines, and see if it works.  If it
still doesn't, what do you get from debug_df_insn?


Bernd
Gerald Pfeifer - July 2, 2010, 8:40 a.m.
On Thu, 1 Jul 2010, H.J. Lu wrote:
> 2010-07-01  H.J. Lu  <hongjiu.lu@intel.com>
> 
> 	PR web/44775
> 	* web.c (union_match_dups): Check NULL *ref.

web is not the proper category for this, is it? ;-)

PR middle-end/44775?

Gerald

Patch

diff --git a/gcc/web.c b/gcc/web.c
index ff91733..4b2ce36 100644
--- a/gcc/web.c
+++ b/gcc/web.c
@@ -123,7 +123,8 @@  union_match_dups (rtx insn, struct web_entry *def_entry,
 	if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
 	  break;
 
-      (*fun) (use_entry + DF_REF_ID (*dupref), entry + DF_REF_ID (*ref));
+      if (*ref)
+	(*fun) (use_entry + DF_REF_ID (*dupref), entry + DF_REF_ID (*ref));
     }
 }