llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.54k stars 11.8k forks source link

Clang uses `__cxa_guard` even in trivial cases where it isn't needed #23427

Open EdSchouten opened 9 years ago

EdSchouten commented 9 years ago
Bugzilla Link 23053
Version trunk
OS All
CC @DougGregor

Extended Description

Consider the following piece of code:

class Bunny {};

Bunny& GetBunny() {
  static Bunny bunny;
  return bunny;
}

When compiled, Clang is smart enough to just emit a static copy of a Bunny object and implement GetBunny() as follows:

0000000000000000 <_Z8GetBunnyv>:
   0:   b8 00 00 00 00          mov    $0x0,%eax
   5:   c3                      retq   

Now consider the case where we add the following trivial constructor to Bunny:

class Bunny {
 public:
  Bunny() {}
};

Suddenly the GetBunny() function starts to look like this:

0000000000000000 <_Z8GetBunnyv>:
   0:   50                      push   %rax
   1:   8a 05 00 00 00 00       mov    0x0(%rip),%al        # 7 <_Z8GetBunnyv+0x7>
   7:   84 c0                   test   %al,%al
   9:   75 18                   jne    23 <_Z8GetBunnyv+0x23>
   b:   bf 00 00 00 00          mov    $0x0,%edi
  10:   e8 00 00 00 00          callq  15 <_Z8GetBunnyv+0x15>
  15:   85 c0                   test   %eax,%eax
  17:   74 0a                   je     23 <_Z8GetBunnyv+0x23>
  19:   bf 00 00 00 00          mov    $0x0,%edi
  1e:   e8 00 00 00 00          callq  23 <_Z8GetBunnyv+0x23>
  23:   b8 00 00 00 00          mov    $0x0,%eax
  28:   5a                      pop    %rdx
  29:   c3                      retq   

This is of course not needed, as the constructor is trivial. There is no need to explicitly invoke it. In fact, if I make the constructor constexpr, Clang is smart enough to reduce GetBunny() to the simple implementation shown above. Clang should be able to use some smartness even if constexpr is not used.

Can reproduce this with the following versions of Clang:

FreeBSD clang version 3.4.1 (tags/RELEASE_34/dot1-final 208032) 20140512
clang version 3.7.0 (trunk 233268)
wheatman commented 9 months ago

This behavior is still the same in post 17 trunk(975deb366470e4943a5b73d8f3031ed54dec6e8b) https://godbolt.org/z/zYEo4Y4WK

code

class Bunny {
 public:
  Bunny() {}
};

Bunny& GetBunny() {
  static Bunny bunny;
  return bunny;
}

class Cat {
    public:
    Cat() = default;
};

Cat& GetCat() {
  static Cat cat;
  return cat;
}

class Puppy {
};

Puppy& GetPuppy() {
  static Puppy puppy;
  return puppy;
}

assembly, notice the different between bunny, can and puppy

GetBunny():                           # @GetBunny()
        movzx   eax, byte ptr [rip + guard variable for GetBunny()::bunny]
        test    al, al
        je      .LBB0_1
        lea     rax, [rip + GetBunny()::bunny]
        ret
.LBB0_1:
        push    rax
        lea     rdi, [rip + guard variable for GetBunny()::bunny]
        call    __cxa_guard_acquire@PLT
        test    eax, eax
        je      .LBB0_3
        lea     rdi, [rip + guard variable for GetBunny()::bunny]
        call    __cxa_guard_release@PLT
.LBB0_3:
        add     rsp, 8
        lea     rax, [rip + GetBunny()::bunny]
        ret
GetCat():                             # @GetCat()
        lea     rax, [rip + GetCat()::cat]
        ret
GetPuppy():                           # @GetPuppy()
        lea     rax, [rip + GetPuppy()::puppy]
        ret
Endilll commented 8 months ago

I should be noted that

class Bunny {
 public:
  Bunny() {}
};

is not a trivially constructible class, despite author claiming that Bunny() {} is a trivial constructor, because it's not. I wonder to which extent our hands are tied by language rules.

llvmbot commented 8 months ago

@llvm/issue-subscribers-c-1

Author: Ed Schouten (EdSchouten)

| | | | --- | --- | | Bugzilla Link | [23053](https://llvm.org/bz23053) | | Version | trunk | | OS | All | | CC | @DougGregor | ## Extended Description Consider the following piece of code: ```cpp class Bunny {}; Bunny& GetBunny() { static Bunny bunny; return bunny; } ``` When compiled, Clang is smart enough to just emit a static copy of a `Bunny` object and implement `GetBunny()` as follows: ```asm 0000000000000000 <_Z8GetBunnyv>: 0: b8 00 00 00 00 mov $0x0,%eax 5: c3 retq ``` Now consider the case where we add the following trivial constructor to Bunny: ```cpp class Bunny { public: Bunny() {} }; ``` Suddenly the `GetBunny()` function starts to look like this: ```asm 0000000000000000 <_Z8GetBunnyv>: 0: 50 push %rax 1: 8a 05 00 00 00 00 mov 0x0(%rip),%al # 7 <_Z8GetBunnyv+0x7> 7: 84 c0 test %al,%al 9: 75 18 jne 23 <_Z8GetBunnyv+0x23> b: bf 00 00 00 00 mov $0x0,%edi 10: e8 00 00 00 00 callq 15 <_Z8GetBunnyv+0x15> 15: 85 c0 test %eax,%eax 17: 74 0a je 23 <_Z8GetBunnyv+0x23> 19: bf 00 00 00 00 mov $0x0,%edi 1e: e8 00 00 00 00 callq 23 <_Z8GetBunnyv+0x23> 23: b8 00 00 00 00 mov $0x0,%eax 28: 5a pop %rdx 29: c3 retq ``` This is of course not needed, as the constructor is trivial. There is no need to explicitly invoke it. In fact, if I make the constructor `constexpr`, Clang is smart enough to reduce `GetBunny()` to the simple implementation shown above. Clang should be able to use some smartness even if `constexpr` is not used. Can reproduce this with the following versions of Clang: ``` FreeBSD clang version 3.4.1 (tags/RELEASE_34/dot1-final 208032) 20140512 clang version 3.7.0 (trunk 233268) ```