diff mbox

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

Message ID AANLkTin6VXuXO24LTcblnCi__37uW3DeVGdZeE5w9ya4@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu July 2, 2010, 12:42 a.m. UTC 
On Thu, Jul 1, 2010 at 4:18 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> 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?
>

I posted  a new patch for PR 44695 at:

http://gcc.gnu.org/ml/gcc-patches/2010-07/msg00099.html

Here is a new patch for PR 44775 to assert *ref != NULL. OK
for trunk?

Thanks.

Comments

Bernd Schmidt July 2, 2010, 8:23 a.m. UTC | #1 
On 07/02/2010 02:42 AM, H.J. Lu wrote:
> 
> Here is a new patch for PR 44775 to assert *ref != NULL. OK
> for trunk?

Doesn't it crash anyway if that assert is false?


Bernd
H.J. Lu July 2, 2010, 2:16 p.m. UTC | #2 
On Fri, Jul 2, 2010 at 1:23 AM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 07/02/2010 02:42 AM, H.J. Lu wrote:
>>
>> Here is a new patch for PR 44775 to assert *ref != NULL. OK
>> for trunk?
>
> Doesn't it crash anyway if that assert is false?
>
>

One is segmentation fault and one is ICE.
diff mbox

Patch

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