[Bug fortran/101824] New: VOLATILE not honored

[Bug fortran/101824] New: VOLATILE not honored

[martin.diehl at kuleuven dot be]

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

            Bug ID: 101824
           Summary: VOLATILE not honored
           Product: gcc
           Version: 11.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: martin.diehl at kuleuven dot be
  Target Milestone: ---

I have a wrapper to C to control a Fortran program with IPC signals:


/* c_interface.c */
#include <signal.h>

void signalterm_c(void (*handler)(int)){
  signal(SIGTERM, handler);
}

void signalusr1_c(void (*handler)(int)){
  signal(SIGUSR1, handler);
}



! c_interface.f90
module c_interface

  implicit none
  interface

  subroutine signalterm_C(handler) bind(C)
    use, intrinsic :: ISO_C_Binding, only: C_FUNPTR

    type(C_FUNPTR), intent(in), value :: handler
  end subroutine signalterm_C

  subroutine signalusr1_C(handler) bind(C)
    use, intrinsic :: ISO_C_Binding, only: C_FUNPTR

    type(C_FUNPTR), intent(in), value :: handler
  end subroutine signalusr1_C

  end interface

end module c_interface


! wait_for_SIGTERM.f90
program wait_for_SIGTERM
  use, intrinsic :: ISO_C_binding

  use c_interface

  implicit none
  logical, volatile :: &
    interface_SIGTERM, &  ! termination signal
    interface_SIGUSR1     ! 1. user-defined signal

  call init

  do while( .not. interface_SIGTERM)


  enddo

contains

subroutine init()

  call signalterm_c(c_funloc(catchSIGTERM))
  call signalusr1_c(c_funloc(catchSIGUSR1))
  call interface_setSIGTERM(.false.)
  call interface_setSIGUSR1(.false.)

end subroutine init


subroutine catchSIGTERM(signal) bind(C)

  integer(C_INT), value :: signal


  print'(a,i0)', ' received signal ',signal
  call interface_setSIGTERM(.not. interface_SIGTERM)

end subroutine catchSIGTERM


subroutine catchSIGUSR1(signal) bind(C)

  integer(C_INT), value :: signal


  print'(a,i0)', ' received signal ',signal
  call interface_setSIGUSR1(.not. interface_SIGUSR1)

end subroutine catchSIGUSR1


subroutine interface_setSIGTERM(state)

  logical, intent(in) :: state


  interface_SIGTERM = state
  print*, 'set SIGTERM to',state

end subroutine interface_setSIGTERM


subroutine interface_setSIGUSR1(state)

  logical, intent(in) :: state


  interface_SIGUSR1 = state
  print*, 'set SIGUSR1 to',state

end subroutine interface_setSIGUSR1

end program



Compile and run (tested on Linux):

gfortran c_interface.c c_interface.f90 wait_for_SIGTERM.f90 -o wait
./wait&
kill -SIGUSR1 XXXX
kill -SIGUSR1 XXXX
kill -SIGTERM XXXX

If compiled without optimization flag, the loop exits on SIGTERM (indented
behavior). If compiled with `-O3`, it runs forever. This behavior is
independent of the VOLATILE attribute.

There is a little discussion on
https://fortran-lang.discourse.group/t/volatile-needed/1648

[Bug tree-optimization/101824] VOLATILE not honored

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
           Keywords|                            |wrong-code
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
          Component|fortran                     |tree-optimization
   Last reconfirmed|                            |2021-08-09

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed, it's predictive commoning that somehow scraps the volatile from the
load.

[Bug tree-optimization/101824] VOLATILE not honored

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
OK, so the underlying issue is that predcom uses the appropriate "GENERIC" way
of assessing if something is volatile:

static bool
suitable_reference_p (struct data_reference *a, enum ref_step_type *ref_step)
{
  tree ref = DR_REF (a), step = DR_STEP (a);

  if (!step
      || TREE_THIS_VOLATILE (ref)
      || !is_gimple_reg_type (TREE_TYPE (ref))
      || tree_could_throw_p (ref))
    return false;

but we have

 <component_ref 0x7ffff635c690
    type <boolean_type 0x7ffff67189d8 logical(kind=4) volatile public unsigned
SI
        size <integer_cst 0x7ffff6543df8 constant 32>
        unit-size <integer_cst 0x7ffff6543e10 constant 4>
        align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7ffff67189d8 precision:1 min <integer_cst 0x7ffff6561168 0> max <integer_cst
0x7ffff6561198 1>>

    arg:0 <var_decl 0x7ffff6367120 FRAME.8
        type <record_type 0x7ffff67210a8 FRAME.wait_for_sigterm BLK
            size <integer_cst 0x7ffff6720780 constant 576>
            unit-size <integer_cst 0x7ffff67207b0 constant 72>
            user align:64 warn_if_not_align:0 symtab:0 alias-set 10
canonical-type 0x7ffff67210a8 fields <field_decl 0x7ffff67167b8
interface_sigterm>
            pointer_to_this <pointer_type 0x7ffff6721150>>
        addressable used ignored BLK /tmp/t.f90:1:24 size <integer_cst
0x7ffff6720780 576> unit-size <integer_cst 0x7ffff67207b0 72>
        align:128 warn_if_not_align:0 context <function_decl 0x7ffff6724000
main> abstract_origin <var_decl 0x7ffff6725a20 FRAME.8>>
    arg:1 <field_decl 0x7ffff67167b8 interface_sigterm type <boolean_type
0x7ffff67189d8 logical(kind=4)>
        volatile unsigned nonaddressable SI /tmp/t.f90:8:21 size <integer_cst
0x7ffff6543df8 32> unit-size <integer_cst 0x7ffff6543e10 4>
        align:32 warn_if_not_align:0 offset_align 128
        offset <integer_cst 0x7ffff6543be8 constant 0>
        bit-offset <integer_cst 0x7ffff6543c30 constant 0> context <record_type
0x7ffff67210a8 FRAME.wait_for_sigterm>
        chain <field_decl 0x7ffff6716720 interface_sigusr1 type <boolean_type
0x7ffff67189d8 logical(kind=4)>
            volatile unsigned nonaddressable SI /tmp/t.f90:9:21 size
<integer_cst 0x7ffff6543df8 32> unit-size <integer_cst 0x7ffff6543e10 4>
            align:32 warn_if_not_align:0 offset_align 128 offset <integer_cst
0x7ffff6543be8 0> bit-offset <integer_cst 0x7ffff6543df8 32> context
<record_type 0x7ffff67210a8 FRAME.wait_for_sigterm> chain <field_decl
0x7ffff6716980 catchsigusr1>>>>

somehow for the corresponding C testcase

int main()
{
  struct X { volatile int y; } z;
  while (z.y)
    ;
  return 0;
}

it "works" but when doing

int main()
{
  volatile int y;
  void bar()
    {
      __builtin_printf ("%d", y);
    }
  while (y)
    ;
  return 0;
}

it reproduces the issue pointing to a bug in tree-nested.c.

[Bug tree-optimization/101824] VOLATILE not honored

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:bb169406cdc9e044eaec500dd742c2fed40f5488

commit r12-2831-gbb169406cdc9e044eaec500dd742c2fed40f5488
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Aug 9 10:19:10 2021 +0200

    middle-end/101824 - properly handle volatiles in nested fn lowering

    When we build the COMPONENT_REF of a formerly volatile local off
    the FRAME decl we have to make sure to mark the COMPONENT_REF
    as TREE_THIS_VOLATILE.  While the GIMPLE operand scanner looks
    at the FIELD_DECL this is not how volatile GENERIC refs work.

    2021-08-09  Richard Biener  <rguenther@suse.de>

            PR middle-end/101824
            * tree-nested.c (get_frame_field): Mark the COMPONENT_REF as
            volatile in case the variable was.

            * gcc.dg/tree-ssa/pr101824.c: New testcase.

[Bug tree-optimization/101824] [9/10/11 Regression] VOLATILE not honored

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |9.5
      Known to fail|                            |11.2.0
           Priority|P3                          |P2
            Summary|VOLATILE not honored        |[9/10/11 Regression]
                   |                            |VOLATILE not honored
      Known to work|                            |12.0

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar.  The issue is latent on branches and a regression from
introduction of predictive commoning (but it could otherwise have
materialized).

[Bug tree-optimization/101824] [9/10/11 Regression] VOLATILE not honored

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:3f29e301f299a1b4e2c535affb964f0b97b7639c

commit r11-8964-g3f29e301f299a1b4e2c535affb964f0b97b7639c
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Aug 9 10:19:10 2021 +0200

    middle-end/101824 - properly handle volatiles in nested fn lowering

    When we build the COMPONENT_REF of a formerly volatile local off
    the FRAME decl we have to make sure to mark the COMPONENT_REF
    as TREE_THIS_VOLATILE.  While the GIMPLE operand scanner looks
    at the FIELD_DECL this is not how volatile GENERIC refs work.

    2021-08-09  Richard Biener  <rguenther@suse.de>

            PR middle-end/101824
            * tree-nested.c (get_frame_field): Mark the COMPONENT_REF as
            volatile in case the variable was.

            * gcc.dg/tree-ssa/pr101824.c: New testcase.

    (cherry picked from commit bb169406cdc9e044eaec500dd742c2fed40f5488)

[Bug tree-optimization/101824] [9/10 Regression] VOLATILE not honored

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:a68a1e06f2eea2f510649aa9bbb17303ef1efb9c

commit r10-10205-ga68a1e06f2eea2f510649aa9bbb17303ef1efb9c
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Aug 9 10:19:10 2021 +0200

    middle-end/101824 - properly handle volatiles in nested fn lowering

    When we build the COMPONENT_REF of a formerly volatile local off
    the FRAME decl we have to make sure to mark the COMPONENT_REF
    as TREE_THIS_VOLATILE.  While the GIMPLE operand scanner looks
    at the FIELD_DECL this is not how volatile GENERIC refs work.

    2021-08-09  Richard Biener  <rguenther@suse.de>

            PR middle-end/101824
            * tree-nested.c (get_frame_field): Mark the COMPONENT_REF as
            volatile in case the variable was.

            * gcc.dg/tree-ssa/pr101824.c: New testcase.

    (cherry picked from commit bb169406cdc9e044eaec500dd742c2fed40f5488)

[Bug tree-optimization/101824] [9 Regression] VOLATILE not honored

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:8dec0ae6200ae8c56dbeef5ff9acd2214282de67

commit r9-9817-g8dec0ae6200ae8c56dbeef5ff9acd2214282de67
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Aug 9 10:19:10 2021 +0200

    middle-end/101824 - properly handle volatiles in nested fn lowering

    When we build the COMPONENT_REF of a formerly volatile local off
    the FRAME decl we have to make sure to mark the COMPONENT_REF
    as TREE_THIS_VOLATILE.  While the GIMPLE operand scanner looks
    at the FIELD_DECL this is not how volatile GENERIC refs work.

    2021-08-09  Richard Biener  <rguenther@suse.de>

            PR middle-end/101824
            * tree-nested.c (get_frame_field): Mark the COMPONENT_REF as
            volatile in case the variable was.

            * gcc.dg/tree-ssa/pr101824.c: New testcase.

    (cherry picked from commit bb169406cdc9e044eaec500dd742c2fed40f5488)

[Bug tree-optimization/101824] [9 Regression] VOLATILE not honored

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED
      Known to fail|                            |9.4.0
      Known to work|                            |9.4.1

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.