[Bug rtl-optimization/64815] New: ICE caused by volatile_refs_p doesn't skip NULL operand

[Bug rtl-optimization/64815] New: ICE caused by volatile_refs_p doesn't skip NULL operand

[terry.guo at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64815

            Bug ID: 64815
           Summary: ICE caused by volatile_refs_p doesn't skip NULL
                    operand
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: terry.guo at arm dot com

I was calling function volatile_refs_p to implement a new feature, and then ran
into below ICE:

f.c: In function 'foo':
f.c:8:1: internal compiler error: Segmentation fault
 }
 ^
0xa52bef crash_signal
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/toplev.c:381
0x9f1edb volatile_refs_p(rtx_def const*)
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/rtlanal.c:2350
0x9f2009 volatile_refs_p(rtx_def const*)
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/rtlanal.c:2389
0x9f2009 volatile_refs_p(rtx_def const*)
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/rtlanal.c:2389
0xefa2fd recog_72
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/config/arm/arm.md:10757
0xefa2fd recog_90
   
/work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/config/arm/arm-fixed.md:230
0x9be96b insn_invalid_p(rtx_insn*, bool)
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/recog.c:357
0x9beb50 verify_changes(int)
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/recog.c:455
0x9bedda apply_change_group()
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/recog.c:548
0xf8eb18 cond_exec_process_if_block
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/ifcvt.c:754
0xf93299 cond_exec_find_if_block
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/ifcvt.c:3762
0xf93299 find_if_header
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/ifcvt.c:3444
0xf94c79 if_convert
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/ifcvt.c:4587
0xf94d4d execute
    /work/terry/build-toolchain/GCC32RM-448/src/gcc/gcc/ifcvt.c:4780

This happens when volatile_refs_p is recursively scanning operands in below
insn:

(gdb) p debug_rtx(x)
(insn 12 11 51 3 (cond_exec (gt (reg:CC 100 cc)
            (const_int 0 [0]))
        (set (reg:SI 0 r0 [orig:110 D.4180 ] [110])
            (mem/v:SI (reg/v/f:SI 2 r2 [orig:114 c ] [114]) [1 *c_5(D)+0 S4
A32]))) f.c:5 -1
     (expr_list:REG_DEAD (reg/v/f:SI 2 r2 [orig:114 c ] [114])
        (nil)))

The last operand (nil) in expr_list caused this ICE. By adding below code, I
can avoid such ICE.

int
volatile_refs_p (const_rtx x)
{
  if (x == 0)
    return 0;

  const RTX_CODE code = GET_CODE (x);
  ........
}

But I am not sure this is the correct fix. Maybe I am calling volatile_refs_p
in wrong way. Please advise.

[Bug rtl-optimization/64815] ICE caused by volatile_refs_p doesn't skip NULL operand

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64815

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I don't think you need to call volatile_refs_p on the notes part of the
instruciton.

[Bug rtl-optimization/64815] ICE caused by volatile_refs_p doesn't skip NULL operand

[terry.guo at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64815

--- Comment #2 from Terry Guo <terry.guo at arm dot com> ---
(In reply to Andrew Pinski from comment #1)
> I don't think you need to call volatile_refs_p on the notes part of the
> instruciton.

The volatile_refs_p works in a recursive way which makes itself to scan the
notes part. I just call volatile_refs_p to the insn.

Do you mean I should update volatile_refs_p to avoid recursively scanning the
notes part of the insn?

[Bug rtl-optimization/64815] ICE caused by volatile_refs_p doesn't skip NULL operand

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64815

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Terry Guo from comment #2)
> (In reply to Andrew Pinski from comment #1)
> > I don't think you need to call volatile_refs_p on the notes part of the
> > instruciton.
> 
> The volatile_refs_p works in a recursive way which makes itself to scan the
> notes part. I just call volatile_refs_p to the insn.
> 
> Do you mean I should update volatile_refs_p to avoid recursively scanning
> the notes part of the insn?

Oh you should be using volatile_refs_p on the insn but only the set; maybe even
just the lhs or rhs of the set.

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Terry Guo from comment #2)
> (In reply to Andrew Pinski from comment #1)
> > I don't think you need to call volatile_refs_p on the notes part of the
> > instruciton.
> 
> The volatile_refs_p works in a recursive way which makes itself to scan the
> notes part. I just call volatile_refs_p to the insn.
> 
> Do you mean I should update volatile_refs_p to avoid recursively scanning
> the notes part of the insn?

Oh you should be using volatile_refs_p on the insn but only the set.

dse.c:      || volatile_refs_p (PATTERN (insn))

[Bug rtl-optimization/64815] ICE caused by volatile_refs_p doesn't skip NULL operand

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64815

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Terry Guo from comment #2)
> (In reply to Andrew Pinski from comment #1)
> > I don't think you need to call volatile_refs_p on the notes part of the
> > instruciton.
> 
> The volatile_refs_p works in a recursive way which makes itself to scan the
> notes part. I just call volatile_refs_p to the insn.
> 
> Do you mean I should update volatile_refs_p to avoid recursively scanning
> the notes part of the insn?

Oh you should be using volatile_refs_p on the insn but only the set; maybe even
just the lhs or rhs of the set.

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Terry Guo from comment #2)
> (In reply to Andrew Pinski from comment #1)
> > I don't think you need to call volatile_refs_p on the notes part of the
> > instruciton.
> 
> The volatile_refs_p works in a recursive way which makes itself to scan the
> notes part. I just call volatile_refs_p to the insn.
> 
> Do you mean I should update volatile_refs_p to avoid recursively scanning
> the notes part of the insn?

Oh you should be using volatile_refs_p on the insn but only the set.

dse.c:      || volatile_refs_p (PATTERN (insn))

[Bug rtl-optimization/64815] ICE caused by volatile_refs_p doesn't skip NULL operand

[terry.guo at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64815

--- Comment #5 from Terry Guo <terry.guo at arm dot com> ---
(In reply to Andrew Pinski from comment #4)
> (In reply to Terry Guo from comment #2)
> > (In reply to Andrew Pinski from comment #1)
> > > I don't think you need to call volatile_refs_p on the notes part of the
> > > instruciton.
> > 
> > The volatile_refs_p works in a recursive way which makes itself to scan the
> > notes part. I just call volatile_refs_p to the insn.
> > 
> > Do you mean I should update volatile_refs_p to avoid recursively scanning
> > the notes part of the insn?
> 
> Oh you should be using volatile_refs_p on the insn but only the set.
> 
> dse.c:      || volatile_refs_p (PATTERN (insn))

Thanks so much. You are right. The ICE disappears. Please mark this bug as
INVALID.

[Bug rtl-optimization/64815] ICE caused by volatile_refs_p doesn't skip NULL operand

[terry.guo at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64815

Terry Guo <terry.guo at arm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #6 from Terry Guo <terry.guo at arm dot com> ---
I am calling the function in wrong way. Thanks Andrew, now everything is ok.