Skip to content

Conversation

@rorth
Copy link
Collaborator

@rorth rorth commented Feb 3, 2025

When running an LLVM 20.1.0 rc1 reltest on Linux/sparc64, all 75 of the new libcxx/test/benchmarks tests FAIL, all in the same way:

# .---command stderr------------
# | Unable to extract number of CPUs.  If your platform uses /proc/cpuinfo, custom support may need to be added.

And indeed the Linux/sparc64 /proc/cpuinfo format is completely different from the x86_64 one: the interesting line is

ncpus active	: 24

This patch adjusts sysinfo.cc to handle that.

Tested on sparc64-unknown-linux-gnu.

I hope this can go in without going via upstream which doesn't have the analogous alpha patch either.

When running an LLVM 20.1.0 rc1 reltest on Linux/sparc64, all 75 of the new
`libcxx/test/benchmarks` tests `FAIL`, all in the same way:

```
```

And indeed the Linux/sparc64 /proc/cpuinfo format is completely different
from the x86_64 one: the interesting line is

ncpus active	: 24

This patch adjusts `sysinfo.cc` to handle that.

Tested on `sparc64-unknown-linux-gnu`.

I hope this can go in without going via upstream which doesn't have the
analogous alpha patch either.
@rorth rorth added this to the LLVM 20.X Release milestone Feb 3, 2025
@rorth rorth requested a review from mtrofin February 3, 2025 14:04
@llvmbot
Copy link
Member

llvmbot commented Feb 3, 2025

@llvm/pr-subscribers-third-party-benchmark

Author: Rainer Orth (rorth)

Changes

When running an LLVM 20.1.0 rc1 reltest on Linux/sparc64, all 75 of the new libcxx/test/benchmarks tests FAIL, all in the same way:

# .---command stderr------------
# | Unable to extract number of CPUs.  If your platform uses /proc/cpuinfo, custom support may need to be added.

And indeed the Linux/sparc64 /proc/cpuinfo format is completely different from the x86_64 one: the interesting line is

ncpus active	: 24

This patch adjusts sysinfo.cc to handle that.

Tested on sparc64-unknown-linux-gnu.

I hope this can go in without going via upstream which doesn't have the analogous alpha patch either.


Full diff: https://github.com/llvm/llvm-project/pull/125503.diff

1 Files Affected:

  • (modified) third-party/benchmark/src/sysinfo.cc (+2)
diff --git a/third-party/benchmark/src/sysinfo.cc b/third-party/benchmark/src/sysinfo.cc
index 2bed1663af2e955..22944a41376f60a 100644
--- a/third-party/benchmark/src/sysinfo.cc
+++ b/third-party/benchmark/src/sysinfo.cc
@@ -520,6 +520,8 @@ int GetNumCPUsImpl() {
   }
 #if defined(__alpha__)
   const std::string Key = "cpus detected";
+#elif defined(__sparc__)
+  const std::string Key = "ncpus active";
 #else
   const std::string Key = "processor";
 #endif

@mtrofin
Copy link
Member

mtrofin commented Feb 3, 2025

Can you please make this change in the google/benchmark repo and link the githash in your commit message - it will help the person that will later sync this fork. Thanks.

@brad0
Copy link
Contributor

brad0 commented Feb 3, 2025

@mtrofin Someone else already submitted a better diff to take care of this by ripping out all the /proc/cpuinfo parsing and using a portable method like every other OS.. google/benchmark@c24774d

@brad0
Copy link
Contributor

brad0 commented Feb 4, 2025

I copied over the commit from upstream here #125603

@rorth rorth removed this from the LLVM 20.X Release milestone Feb 4, 2025
@rorth
Copy link
Collaborator Author

rorth commented Feb 4, 2025

@mtrofin Someone else already submitted a better diff to take care of this by ripping out all the /proc/cpuinfo parsing and using a portable method like every other OS.. google/benchmark@c24774d

Right: I noticed this when preparing the upstream PR for this patch. I'd inadvertently checked my github clone of the benchmark repo rather than upstream itself ;-(

@rorth
Copy link
Collaborator Author

rorth commented Feb 4, 2025

Dropped in favor of PR #125603.

@rorth rorth closed this Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Needs Triage

Development

Successfully merging this pull request may close these issues.

4 participants