Bug 109967 - [12/13/14/15 Regression] Wrong code at -O2 on x86_64-linux-gnu
Summary: [12/13/14/15 Regression] Wrong code at -O2 on x86_64-linux-gnu
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 14.0
: P2 normal
Target Milestone: 11.5
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks: 111843
  Show dependency treegraph
 
Reported: 2023-05-25 14:15 UTC by Shaohua Li
Modified: 2024-11-03 06:13 UTC (History)
6 users (show)

See Also:
Host:
Target: x86_64-linux-gnu
Build:
Known to work: 6.1.0
Known to fail: 7.1.0
Last reconfirmed: 2023-05-26 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Shaohua Li 2023-05-25 14:15:54 UTC
For the following code, gcc trunk at -O2 emits the wrong code. This seems to be a long latent bug since GCC-7 (I tried gcc-7.4 on compiler explorer, which still emits the wrong result.)

Compiler explorer: https://godbolt.org/z/YGeWedWq7

$ cat a.c
int printf(const char *, ...);
int a, c;
int main() {
  long f[2];
  int e = 0;
  for (; e < 2; e++) {
    {
      char g[20];
      char *b = g;
      int d = 48, e = 0;
      while (d && e < 5)
        b[e++] = d /= 10;
      c = e;
    }
    f[c - 2 + e] = 1;
  }
  a = f[0];
  printf("%d\n", a);
}
$
$ gcc-tk -O0 a.c && ./a.out
1
$ gcc-tk -O2 a.c && ./a.out
4
$
$ gcc-tk -v
Using built-in specs.
COLLECT_GCC=gcc-tk
COLLECT_LTO_WRAPPER=/zdata/shaoli/compilers/ccbuilder-compilers/gcc-8d5f050dabbf6dd3b992c3b46661848dbcf30d9e/libexec/gcc/x86_64-pc-linux-gnu/14.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../configure --disable-multilib --disable-bootstrap --enable-languages=c,c++ --prefix=/zdata/shaoli/compilers/ccbuilder-compilers/gcc-8d5f050dabbf6dd3b992c3b46661848dbcf30d9e
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 14.0.0 20230523 (experimental) (GCC) 
$
Comment 1 Andrew Pinski 2023-05-25 18:16:48 UTC
So if I initialize f the code works.

Note the gimple level code looks the same.
Comment 2 Andrew Pinski 2023-05-25 18:19:20 UTC
The problem is the Partition code in expand.

Without the ={0,0}, we get:
Partition 1: size 20 align 16
	g	f

With we get:
Partition 1: size 20 align 16
	g
Partition 0: size 16 align 16
	f
Comment 3 Davin McCall 2023-06-05 11:13:35 UTC
-fdisable-tree-cunroll avoids it.

This is the take of someone who is new to poking GCC internals, so take this with a grain of salt, but:

It looks like cunroll duplicates (partially unrolls, I suppose) the inner loop, meaning that `g' is live in two disjoint ranges, with a CLOBBER between them.

Then later passes (fre5, but disabling it doesn't seem to help) recognise that &g and &g are the same in both ranges and so use a single temporary for both. This confuses cfgexpand (as Andrew Pinski notes) because the memory dereference of the temporary isn't seen as an access of g (in add_scope_conflicts()/add_scope_conflicts_1()/visit_conflict()). 

I don't understand the IR semantics well enough to know the right fix - perhaps cunroll should be removing the clobber (does a clobber affect storage lifetime or only value?), or perhaps cfgexpand should be more conservative when it sees a memory access.
Comment 4 Alexander Monakov 2023-06-05 15:39:49 UTC
See PR 90348 for the discussion of problematic lifetime semantics.
Comment 5 Richard Biener 2023-07-07 10:45:28 UTC
GCC 10 branch is being closed.
Comment 6 Shaohua Li 2023-07-28 11:47:08 UTC
Bisected to r9-2635-g78ea9abc201
Comment 7 Shaohua Li 2023-09-25 12:59:31 UTC
This test case does not reproduce anymore on the current trunk. Maybe one of the recent fixes fixed the underlying issue as well.
Comment 8 Xi Ruoyao 2023-09-25 15:02:01 UTC
(In reply to Shaohua Li from comment #7)
> This test case does not reproduce anymore on the current trunk. Maybe one of
> the recent fixes fixed the underlying issue as well.

But we still need to ensure the fix backported into 11/12/13.  And there is still a chance that the issue might be covered up by an unrelated change.
Comment 9 Xi Ruoyao 2023-09-25 17:27:26 UTC
Bisect shows r14-4089 (the fix for PR111294) either fixes or "covers up" the issue.
Comment 10 Jakub Jelinek 2024-02-08 18:54:18 UTC
Plus we have the r14-7274 workaround on the trunk now, wouldn't that make the problem go away too?
Comment 11 Richard Biener 2024-07-19 13:20:34 UTC
GCC 11 branch is being closed.
Comment 12 Andrew Pinski 2024-10-03 04:28:17 UTC
This was fixed by the same patch which fixed PR 111422.

  ivtmp.19_7 = (unsigned long) &g;
...


  <bb 4> [local count: 89482957]:
  g ={v} {CLOBBER(eol)};
  _51 = e_16 + -1;
  f[_51] = 1;

  <bb 5> [local count: 447307436]:
  # d_36 = PHI <d_25(5), 48(4)>
  # e_37 = PHI <e_26(5), 0(4)>
  # ivtmp.14_28 = PHI <ivtmp.14_24(5), ivtmp.19_7(4)>
...


(same BB):
  # ivtmp.14_28 = PHI <ivtmp.14_24(5), ivtmp.19_7(4)>
...
  MEM[(char *)_55] = _3;


So we now know that g is alive at the same time f is alive so we know they conflict.