llvm / llvm-project

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

Should undefined symbols in msvc stl(_Literal_zero_is_expected) be processed in IR generation or sema #97225

Open GkvJwa opened 2 months ago

GkvJwa commented 2 months ago

Found that using msvc's stl to compile chromium(https://github.com/microsoft/STL/issues/4359#issuecomment-2042911928)

The cause of the error is as described in this link(https://github.com/oneapi-src/oneDPL/pull/1568#issue-2283898641), mscv(cl.exe) will optimize the if expr here, therefore cef specially patched chromum(https://github.com/chromiumembedded/cef/commit/cb1830e16ccf54cfb676933c35d189864f02a276), make the compilation pass

But recently I continued to analyze this compilation error.

I found that it was caused

1.use default operator
friend constexpr std::strong_ordering operator<=>(A, A) = default;
2.called
static_assert(A a < A b, "");

so I reproduced it with the following code

$ cat a.cpp
#include <stdint.h>
#include <compare>
#include <limits>

namespace base {

class TimeDelta {
 public:
  constexpr TimeDelta() = default;
  constexpr explicit TimeDelta(int64_t delta_us) : delta_(delta_us) {}

  // _Literal_zero_is_expected
  //constexpr bool operator<(TimeDelta other) const;

  friend constexpr std::strong_ordering operator<=>(TimeDelta,
                                                    TimeDelta) = default;

 private:
  int64_t delta_ = 0;
};

// constexpr bool TimeDelta::operator<(TimeDelta other) const {
//   return delta_ < other.delta_;
// }

}  // namespace base

static_assert(base::TimeDelta(2) < base::TimeDelta(100), "");

class Foo {
 public:
  void foo1() { foo2(base::TimeDelta(1)); }

 private:
  void foo2(base::TimeDelta wait_delta) {
    if (wait_delta <= base::TimeDelta()) {
      return;
    }
  }

  base::TimeDelta b_;
};

int main() {
  Foo foo;
  foo.foo1();
  return 0;
}
$ clang-cl.exe -cc1 -emit-obj -internal-isystem "C:\\Program Files\\Microsoft Visual Studio\\2022\\Professional\\VC\\Tools\\MSVC\\14.40.33807\\include" -internal-isystem "C:\\Program Files (x86)\\Windows Kits\\10\\Include\\10.0.22621.0\\ucrt" -fms-extensions -std=c++20 -o "a.obj" "a.cpp"

What is very interesting is that if override operator<(commented code)

constexpr bool operator<(TimeDelta other) const;

will not generate the symbols of std::_Literal_zero::_Literal_zero in base::operator<=>

For this reason, I debugged and found that it was not InImmediateFunctionContext https://github.com/llvm/llvm-project/blob/4066a3206012cded6de2e286732dca721d37bcd3/clang/include/clang/Sema/Sema.h#L10309-L10315 of course, it also means that there will be no compilation error before this commit

SHA-1: a7a74ece1d6b8dc95bf6fb0b7e06b8e0ba9b9559

* [Clang] Fixes to immediate-escalating functions (#82281)

After that, without overriding the operator, I want to optimize this expression

if (_Zero != 0) {
    _Literal_zero_is_expected();
}

But reading code, it is not very good to process it in buildIfStmt, so should it be processed in IR Gen?

llvmbot commented 2 months ago

@llvm/issue-subscribers-clang-frontend

Author: None (GkvJwa)

Found that using msvc's stl to compile chromium(https://github.com/microsoft/STL/issues/4359#issuecomment-2042911928) The cause of the error is as described in this link(https://github.com/oneapi-src/oneDPL/pull/1568#issue-2283898641), mscv(cl.exe) will optimize the if expr here, therefore cef specially patched chromum(https://github.com/chromiumembedded/cef/commit/cb1830e16ccf54cfb676933c35d189864f02a276), make the compilation pass But recently I continued to analyze this compilation error. I found that it was caused ``` 1.use default operator friend constexpr std::strong_ordering operator<=>(A, A) = default; 2.called static_assert(A a < A b, ""); ``` so I reproduced it with the following code ``` $ cat a.cpp #include <stdint.h> #include <compare> #include <limits> namespace base { class TimeDelta { public: constexpr TimeDelta() = default; constexpr explicit TimeDelta(int64_t delta_us) : delta_(delta_us) {} // _Literal_zero_is_expected //constexpr bool operator<(TimeDelta other) const; friend constexpr std::strong_ordering operator<=>(TimeDelta, TimeDelta) = default; private: int64_t delta_ = 0; }; // constexpr bool TimeDelta::operator<(TimeDelta other) const { // return delta_ < other.delta_; // } } // namespace base static_assert(base::TimeDelta(2) < base::TimeDelta(100), ""); class Foo { public: void foo1() { foo2(base::TimeDelta(1)); } private: void foo2(base::TimeDelta wait_delta) { if (wait_delta <= base::TimeDelta()) { return; } } base::TimeDelta b_; }; int main() { Foo foo; foo.foo1(); return 0; } ``` ``` $ clang-cl.exe -cc1 -emit-obj -internal-isystem "C:\\Program Files\\Microsoft Visual Studio\\2022\\Professional\\VC\\Tools\\MSVC\\14.40.33807\\include" -internal-isystem "C:\\Program Files (x86)\\Windows Kits\\10\\Include\\10.0.22621.0\\ucrt" -fms-extensions -std=c++20 -o "a.obj" "a.cpp" ``` What is very interesting is that if override operator<(commented code) ``` constexpr bool operator<(TimeDelta other) const; ``` will not generate the symbols of std::_Literal_zero::_Literal_zero in base::operator<=> For this reason, I debugged and found that it was not InImmediateFunctionContext https://github.com/llvm/llvm-project/blob/4066a3206012cded6de2e286732dca721d37bcd3/clang/include/clang/Sema/Sema.h#L10309-L10315 of course, it also means that there will be no compilation error before this commit ``` SHA-1: a7a74ece1d6b8dc95bf6fb0b7e06b8e0ba9b9559 * [Clang] Fixes to immediate-escalating functions (#82281) ``` After that, without overriding the operator, I want to optimize this expression ``` if (_Zero != 0) { _Literal_zero_is_expected(); } ``` But reading code, it is not very good to process it in buildIfStmt, so should it be processed in IR Gen?
GkvJwa commented 2 months ago

Can help me look at this problem(@sdkrystian @erichkeane @katzdm @cor3ntin) How can I prevent this code (_Literal_zero_is_expected) from being generated when the code is valid

shafik commented 2 months ago

CC @cor3ntin

GkvJwa commented 1 month ago

Add @dtcxzyw

dtcxzyw commented 1 month ago

cc @frederick-vs-ja