fpetrogalli added inline comments.
================
Comment at: llvm/unittests/Support/Host.cpp:33
-class HostTest : public testing::Test {
- Triple Host;
-
-protected:
- bool isSupportedArchAndOS() {
- // Initially this is only testing detection of the number of
- // physical cores, which is currently only supported/tested on
- // some systems.
- return (Host.isOSWindows() && llvm_is_multithreaded()) ||
- Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
- (Host.isOSLinux() && !Host.isAndroid()) ||
- (Host.isSystemZ() && Host.isOSzOS());
- }
-
- HostTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
-};
-
-TEST_F(HostTest, NumPhysicalCoresSupported) {
- if (!isSupportedArchAndOS())
- GTEST_SKIP();
- int Num = sys::getHostNumPhysicalCores();
- ASSERT_GT(Num, 0);
-}
-
-TEST_F(HostTest, NumPhysicalCoresUnsupported) {
- if (isSupportedArchAndOS())
- GTEST_SKIP();
- int Num = sys::getHostNumPhysicalCores();
- ASSERT_EQ(Num, -1);
-}
+class HostTest : public testing::Test {};
----------------
lenary wrote:
> fpetrogalli wrote:
> > Is this needed?
> My original intention was to do the minimal changes required to just move the
> code.
>
> It's not technically needed, so I will remove it.
Yeah, I totally get the idea of doing minimal changes. Juts mention in the
commit message that you have some extra cleanup that is not strictly needed for
the major changes of the patch, so that people blaming the change can
understand why that was done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137836/new/
https://reviews.llvm.org/D137836
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
