[Bug tree-optimization/115278] New: -ftree-vectorize optimizes away volatile write on x86_64

[Bug tree-optimization/115278] New: -ftree-vectorize optimizes away volatile write on x86_64

[raffeck at cs dot fau.de]

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

            Bug ID: 115278
           Summary: -ftree-vectorize optimizes away volatile write on
                    x86_64
           Product: gcc
           Version: 14.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: raffeck at cs dot fau.de
  Target Milestone: ---

Created attachment 58308
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58308&action=edit
minimal example to reproduce bug

The code in the attached minimal example contains two volatile writes to the
same address.

With `gcc -O2 -fno-tree-loop-vectorize`, two writes are present in the
generated object code. With just `gcc -O2` however, only one write is
preserved, the other seems to be optimized away. As these are volatile
accesses, this should be an illegal optimization.

The behavior does not occur with commit 498ad738690, but occurs with
25413fdb2ac, so probably 25413fdb2ac introduced this bug. The behavior still
persists in the current master (commit f46eaad445e).

[Bug tree-optimization/115278] [13/14/15 Regression] -ftree-vectorize optimizes away volatile write on x86_64

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|-ftree-vectorize optimizes  |[13/14/15 Regression]
                   |away volatile write on      |-ftree-vectorize optimizes
                   |x86_64                      |away volatile write on
                   |                            |x86_64
   Target Milestone|---                         |13.4
   Last reconfirmed|                            |2024-05-29
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.
ifcvt removes it:
We go from:
```
  rt.D.2911.value_low = _25;
  rt.D.2911.value_high = _26;
  rt.D.2906.a = 99;
  rt.D.2906.b = 1;
  _32 = rt.D.2911.value_low;
  *WRITE.0_1 ={v} _32;
  _34 = rt.D.2911.value_high;
  *WRITE.0_1 ={v} _34;
```
to:
```
  rt.D.2911.value_low = _2;
  rt.D.2911.value_high = _3;
  _ifc__41 = rt.D.2906.D.2905;
  _ifc__42 = BIT_INSERT_EXPR <_ifc__41, 99, 0 (17 bits)>;
  _ifc__40 = BIT_INSERT_EXPR <_ifc__42, 1, 56 (8 bits)>;
  _44 = BIT_FIELD_REF <_ifc__42, 32, 0>;
  _46 = BIT_FIELD_REF <_ifc__40, 32, 32>;
  *WRITE.0_1 ={v} _46;
```

[Bug tree-optimization/115278] [13/14/15 Regression] -ftree-vectorize optimizes away volatile write on x86_64 since r13-3219

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[13/14/15 Regression]       |[13/14/15 Regression]
                   |-ftree-vectorize optimizes  |-ftree-vectorize optimizes
                   |away volatile write on      |away volatile write on
                   |x86_64                      |x86_64 since r13-3219
                 CC|                            |pinskia at gcc dot gnu.org

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Yes I was going to point to r13-3219-g25413fdb2ac249 also.

[Bug tree-optimization/115278] [13/14/15 Regression] -ftree-vectorize optimizes away volatile write on x86_64 since r13-3219

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think we fixed similar bug on the read side.

[Bug tree-optimization/115278] [13/14/15 Regression] -ftree-vectorize optimizes away volatile write on x86_64 since r13-3219

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
It's actually a latent issue, unrelated to bitfields?  We elide the store via

          tree lhs = gimple_get_lhs (stmt);
          ao_ref write;
          ao_ref_init (&write, lhs);

          if (dse_classify_store (&write, stmt, false, NULL, NULL, latch_vdef)
              == DSE_STORE_DEAD)
            delete_dead_or_redundant_assignment (&gsi, "dead");

but that fails to guard against volatiles.

[Bug tree-optimization/115278] [13/14/15 Regression] -ftree-vectorize optimizes away volatile write on x86_64 since r13-3219

[avieira at gcc dot gnu.org]

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

avieira at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |avieira at gcc dot gnu.org

--- Comment #5 from avieira at gcc dot gnu.org ---
> I think we fixed similar bug on the read side.

I don't have the best memory, but the one I can remember is PR 111882, where we
had the SAVE_EXPR. And the the fix was to not lower bitfields with non-constant
offsets.

Should dse_classify_store not return *_DEAD for volatiles?

[Bug tree-optimization/115278] [13/14/15 Regression] -ftree-vectorize optimizes away volatile write on x86_64 since r13-3219

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to avieira from comment #5)
> > I think we fixed similar bug on the read side.
> 
> I don't have the best memory, but the one I can remember is PR 111882, where
> we had the SAVE_EXPR. And the the fix was to not lower bitfields with
> non-constant offsets.
> 
> Should dse_classify_store not return *_DEAD for volatiles?

It's a low-level worker, it relies on the caller to have performed sanity
checking on the stmt itself.  I'm testing a patch doing that.

[Bug tree-optimization/115278] [13/14/15 Regression] -ftree-vectorize optimizes away volatile write on x86_64 since r13-3219

[pinskia at gcc dot gnu.org]

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

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to avieira from comment #5)
> > I think we fixed similar bug on the read side.
> 
> I don't have the best memory, but the one I can remember is PR 111882, where
> we had the SAVE_EXPR. And the the fix was to not lower bitfields with
> non-constant offsets.

The one which Richard was thinking of was PR 114197 which I linked in the see
also.

[Bug tree-optimization/115278] [13/14/15 Regression] -ftree-vectorize optimizes away volatile write on x86_64 since r13-3219

[avieira at gcc dot gnu.org]

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

--- Comment #8 from avieira at gcc dot gnu.org ---
Thanks! Missed that Andrew.

> It's a low-level worker, it relies on the caller to have performed sanity
> checking on the stmt itself.  I'm testing a patch doing that.

OK, no strong opinion here, but do you reckon we should somehow make that
clearer in terms of comments for this function? I was trying to think of
whether there's a data-flow based reason to ever consider a write of a volatile
ok for elimination, which is how I'd first interpret the meaning of the
classification of this function. But I can't.

[Bug tree-optimization/115278] [13/14/15 Regression] -ftree-vectorize optimizes away volatile write on x86_64 since r13-3219

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from GCC 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:65dbe0ab7cdaf2aa84b09a74e594f0faacf1945c

commit r15-941-g65dbe0ab7cdaf2aa84b09a74e594f0faacf1945c
Author: Richard Biener <rguenther@suse.de>
Date:   Fri May 31 10:14:25 2024 +0200

    tree-optimization/115278 - fix DSE in if-conversion wrt volatiles

    The following adds the missing guard for volatile stores to the
    embedded DSE in the loop if-conversion pass.

            PR tree-optimization/115278
            * tree-if-conv.cc (ifcvt_local_dce): Do not DSE volatile stores.

            * g++.dg/vect/pr115278.cc: New testcase.

[Bug tree-optimization/115278] [13/14 Regression] -ftree-vectorize optimizes away volatile write on x86_64 since r13-3219

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[13/14/15 Regression]       |[13/14 Regression]
                   |-ftree-vectorize optimizes  |-ftree-vectorize optimizes
                   |away volatile write on      |away volatile write on
                   |x86_64 since r13-3219       |x86_64 since r13-3219
      Known to work|                            |15.0

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar.

[Bug tree-optimization/115278] [13/14 Regression] -ftree-vectorize optimizes away volatile write on x86_64 since r13-3219

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:272e8c90af527fc1d0055ad0f17f1d97bb0bd6cb

commit r14-10335-g272e8c90af527fc1d0055ad0f17f1d97bb0bd6cb
Author: Richard Biener <rguenther@suse.de>
Date:   Fri May 31 10:14:25 2024 +0200

    tree-optimization/115278 - fix DSE in if-conversion wrt volatiles

    The following adds the missing guard for volatile stores to the
    embedded DSE in the loop if-conversion pass.

            PR tree-optimization/115278
            * tree-if-conv.cc (ifcvt_local_dce): Do not DSE volatile stores.

            * g++.dg/vect/pr115278.cc: New testcase.

    (cherry picked from commit 65dbe0ab7cdaf2aa84b09a74e594f0faacf1945c)