Bug 102829 - Redundant null check after atomic load from that address
Summary: Redundant null check after atomic load from that address
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 12.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks: VRP
  Show dependency treegraph
 
Reported: 2021-10-19 08:17 UTC by Laurynas Biveinis
Modified: 2021-10-26 00:01 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-10-19 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Laurynas Biveinis 2021-10-19 08:17:58 UTC
The following source code
----
struct d {
  long b;

  d *e() {
    __atomic_load_n(&b, 0);
    return this;
  }
};

d *j;

void i();

void k() {
  auto l = j->e();
  if (l)
    i();
}
----
produces the following bit of assembly with -O3 on x86_64 (https://godbolt.org/z/G1ThvnMWP):
...
        mov     rdx, QWORD PTR [rax]
        test    rax, rax
        je      .L1
...

It first dereferences the address at RAX, and later checks whether RAX == 0. Since we already tried accessing memory at that address, the nullptr check seems redundant. Moreover, since the address happens to be of 'this' of variable j, it being equal to nullptr is UB anyway and may be assumed to be != nullptr?

Clang tests show the redundant test being present up to version 11 inclusive (https://godbolt.org/z/rT3ha4enb) and absent from version 12 onwards (https://godbolt.org/z/Gfb9EWMfq)
Comment 1 Richard Biener 2021-10-19 08:31:32 UTC
We don't derive non-nullness for the pointer dereferenced by the atomic builtins yet.
Comment 2 Laurynas Biveinis 2021-10-19 14:03:35 UTC
FWIW adding "if (this == nullptr) __builtin_unreachable();" between __atomic_load_n and return fails to workaround this issue