Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV] Add support for getHostCPUFeatures using hwprobe #94352

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Jun 4, 2024

Co-authored-by: Yangyu Chen <cyy@cyyself.name>
@tschuett
Copy link
Member

tschuett commented Jun 4, 2024

Do you offer an alternative/default implementation when __NR_riscv_hwprobe is not available, e.g., in docker containers?

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Jun 5, 2024

Do you offer an alternative/default implementation when __NR_riscv_hwprobe is not available, e.g., in docker containers?

Is /proc/cpuinfo available in containers? If so, I can provide a fallback implementation by reading the isa field from this file.

@cyyself
Copy link

cyyself commented Jun 5, 2024

Do you offer an alternative/default implementation when __NR_riscv_hwprobe is not available, e.g., in docker containers?

Is /proc/cpuinfo available in containers? If so, I can provide a fallback implementation by reading the isa field from this file.

I think it is a bad idea. Some vendor custom kernels, such as t-head kernel, regard xtheadvector as v and printed in /proc/cpuinfo. A vast number of RISC-V devices still use these kernels.

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Jun 10, 2024

Ping.

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jun 10, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jun 10, 2024

@llvm/pr-subscribers-backend-risc-v
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch adds support for sys::getHostCPUFeatures using the RISC-V hardware probing interface.
References:

Co-authored-by: @cyyself


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Arch/RISCV.cpp (+7-1)
  • (modified) llvm/lib/TargetParser/Host.cpp (+69)
diff --git a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index 2e2bce8494672..bf34b6e3f2ea4 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -86,8 +86,14 @@ void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
   // and other features (ex. mirco architecture feature) from mcpu
   if (Arg *A = Args.getLastArg(options::OPT_mcpu_EQ)) {
     StringRef CPU = A->getValue();
-    if (CPU == "native")
+    if (CPU == "native") {
       CPU = llvm::sys::getHostCPUName();
+      llvm::StringMap<bool> HostFeatures;
+      if (llvm::sys::getHostCPUFeatures(HostFeatures))
+        for (auto &F : HostFeatures)
+          Features.push_back(
+              Args.MakeArgString((F.second ? "+" : "-") + F.first()));
+    }
 
     getRISCFeaturesFromMcpu(D, A, Triple, CPU, Features);
   }
diff --git a/llvm/lib/TargetParser/Host.cpp b/llvm/lib/TargetParser/Host.cpp
index 68155acd9e5bc..b99e1443ef5e5 100644
--- a/llvm/lib/TargetParser/Host.cpp
+++ b/llvm/lib/TargetParser/Host.cpp
@@ -1998,6 +1998,75 @@ bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
 
   return true;
 }
+#elif defined(__linux__) && defined(__riscv)
+// struct riscv_hwprobe
+struct RISCVHwProbe {
+  int64_t Key;
+  uint64_t Value;
+};
+bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
+  RISCVHwProbe Query[]{{/*RISCV_HWPROBE_KEY_BASE_BEHAVIOR=*/3, 0},
+                       {/*RISCV_HWPROBE_KEY_IMA_EXT_0=*/4, 0}};
+  int Ret = syscall(/*__NR_riscv_hwprobe=*/258, /*pairs=*/Query,
+                    /*pair_count=*/std::size(Query), /*cpu_count=*/0,
+                    /*cpus=*/0, /*flags=*/0);
+  if (Ret != 0)
+    return false;
+
+  uint64_t BaseMask = Query[0].Value;
+  // Check whether RISCV_HWPROBE_BASE_BEHAVIOR_IMA is set.
+  if (BaseMask & 1) {
+    Features["i"] = true;
+    Features["m"] = true;
+    Features["a"] = true;
+  }
+
+  uint64_t ExtMask = Query[1].Value;
+  Features["f"] = ExtMask & (1 << 0);           // RISCV_HWPROBE_IMA_FD
+  Features["d"] = ExtMask & (1 << 0);           // RISCV_HWPROBE_IMA_FD
+  Features["c"] = ExtMask & (1 << 1);           // RISCV_HWPROBE_IMA_C
+  Features["v"] = ExtMask & (1 << 2);           // RISCV_HWPROBE_IMA_V
+  Features["zba"] = ExtMask & (1 << 3);         // RISCV_HWPROBE_EXT_ZBA
+  Features["zbb"] = ExtMask & (1 << 4);         // RISCV_HWPROBE_EXT_ZBB
+  Features["zbs"] = ExtMask & (1 << 5);         // RISCV_HWPROBE_EXT_ZBS
+  Features["zicboz"] = ExtMask & (1 << 6);      // RISCV_HWPROBE_EXT_ZICBOZ
+  Features["zbc"] = ExtMask & (1 << 7);         // RISCV_HWPROBE_EXT_ZBC
+  Features["zbkb"] = ExtMask & (1 << 8);        // RISCV_HWPROBE_EXT_ZBKB
+  Features["zbkc"] = ExtMask & (1 << 9);        // RISCV_HWPROBE_EXT_ZBKC
+  Features["zbkx"] = ExtMask & (1 << 10);       // RISCV_HWPROBE_EXT_ZBKX
+  Features["zknd"] = ExtMask & (1 << 11);       // RISCV_HWPROBE_EXT_ZKND
+  Features["zkne"] = ExtMask & (1 << 12);       // RISCV_HWPROBE_EXT_ZKNE
+  Features["zknh"] = ExtMask & (1 << 13);       // RISCV_HWPROBE_EXT_ZKNH
+  Features["zksed"] = ExtMask & (1 << 14);      // RISCV_HWPROBE_EXT_ZKSED
+  Features["zksh"] = ExtMask & (1 << 15);       // RISCV_HWPROBE_EXT_ZKSH
+  Features["zkt"] = ExtMask & (1 << 16);        // RISCV_HWPROBE_EXT_ZKT
+  Features["zvbb"] = ExtMask & (1 << 17);       // RISCV_HWPROBE_EXT_ZVBB
+  Features["zvbc"] = ExtMask & (1 << 18);       // RISCV_HWPROBE_EXT_ZVBC
+  Features["zvkb"] = ExtMask & (1 << 19);       // RISCV_HWPROBE_EXT_ZVKB
+  Features["zvkg"] = ExtMask & (1 << 20);       // RISCV_HWPROBE_EXT_ZVKG
+  Features["zvkned"] = ExtMask & (1 << 21);     // RISCV_HWPROBE_EXT_ZVKNED
+  Features["zvknha"] = ExtMask & (1 << 22);     // RISCV_HWPROBE_EXT_ZVKNHA
+  Features["zvknhb"] = ExtMask & (1 << 23);     // RISCV_HWPROBE_EXT_ZVKNHB
+  Features["zvksed"] = ExtMask & (1 << 24);     // RISCV_HWPROBE_EXT_ZVKSED
+  Features["zvksh"] = ExtMask & (1 << 25);      // RISCV_HWPROBE_EXT_ZVKSH
+  Features["zvkt"] = ExtMask & (1 << 26);       // RISCV_HWPROBE_EXT_ZVKT
+  Features["zfh"] = ExtMask & (1 << 27);        // RISCV_HWPROBE_EXT_ZFH
+  Features["zfhmin"] = ExtMask & (1 << 28);     // RISCV_HWPROBE_EXT_ZFHMIN
+  Features["zihintntl"] = ExtMask & (1 << 29);  // RISCV_HWPROBE_EXT_ZIHINTNTL
+  Features["zvfh"] = ExtMask & (1 << 30);       // RISCV_HWPROBE_EXT_ZVFH
+  Features["zvfhmin"] = ExtMask & (1ULL << 31); // RISCV_HWPROBE_EXT_ZVFHMIN
+  Features["zfa"] = ExtMask & (1ULL << 32);     // RISCV_HWPROBE_EXT_ZFA
+  Features["ztso"] = ExtMask & (1ULL << 33);    // RISCV_HWPROBE_EXT_ZTSO
+  Features["zacas"] = ExtMask & (1ULL << 34);   // RISCV_HWPROBE_EXT_ZACAS
+  Features["zicond"] = ExtMask & (1ULL << 35);  // RISCV_HWPROBE_EXT_ZICOND
+  Features["zihintpause"] =
+      ExtMask & (1ULL << 36); // RISCV_HWPROBE_EXT_ZIHINTPAUSE
+
+  // TODO: set unaligned-scalar-mem if RISCV_HWPROBE_KEY_MISALIGNED_PERF returns
+  // RISCV_HWPROBE_MISALIGNED_FAST.
+
+  return true;
+}
 #else
 bool sys::getHostCPUFeatures(StringMap<bool> &Features) { return false; }
 #endif

CPU = llvm::sys::getHostCPUName();
llvm::StringMap<bool> HostFeatures;
if (llvm::sys::getHostCPUFeatures(HostFeatures))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open discussion here: CPU may fail and return generic. Should we failback to use getHostCPUFeatures if getHostCPUName fails? Or we should use getHostCPUFeatures all the time?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think hwprobe is more reliable than cpu name because some RV cpus are not recognized by LLVM (e.g., T-head's cores). However, cpu name is still useful even if getHostCPUFeatures returns true because it may provide scheduling model.

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Jun 22, 2024

I have no idea about why it corrupts StringMap. Sad :(

image

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Jun 24, 2024

See #96465

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Jun 26, 2024

Any thoughts? Hopefully I can catch up with the 19 release :)

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@preames preames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM w/minor comments

llvm/lib/TargetParser/Host.cpp Outdated Show resolved Hide resolved
llvm/lib/TargetParser/Host.cpp Show resolved Hide resolved
bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
RISCVHwProbe Query[]{{/*RISCV_HWPROBE_KEY_BASE_BEHAVIOR=*/3, 0},
{/*RISCV_HWPROBE_KEY_IMA_EXT_0=*/4, 0}};
int Ret = syscall(/*__NR_riscv_hwprobe=*/258, /*pairs=*/Query,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider this a purely stylistic comment.

We should probably be using either the vDSO symbol or the glibc shim. In either case, we'd have a weak symbol which could possibly be nullptr, and need to return early.

In this use case, the difference likely doesn't matter, but if we reuse this code, the lack of caching provided by vDSO could be problematic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently sys::getHostCPUFeatures has three callers:

  • clang -> riscv::getRISCVTargetFeatures
  • llvm-tools -> codegen::getFeaturesStr
  • JIT users -> JITTargetMachineBuilder::detectHost

I don't think there are any opportunities to reuse the result.
BTW, #85790 may benefit from the vDSO symbol, but it implements caching itself.

I didn't use the glibc call __riscv_hwprobe since sys/hwprobe.h was unavailable on my RV board :(

for (auto &F : HostFeatures)
Features.push_back(
Args.MakeArgString((F.second ? "+" : "-") + F.first()));
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to update riscv::getRISCVArch? There's analogous logic there for getting features from mcpu native.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wangpc-pp @topperc
Are there any equivalents of the helper printMArch?

// We can generate march string from target features as what has been described
// in RISC-V ISA specification (version 20191213) 'Chapter 27. ISA Extension
// Naming Conventions'.
//
// This is almost the same as RISCVFeatures::parseFeatureBits, except that we
// get feature name from feature records instead of feature bits.
static void printMArch(raw_ostream &OS, const std::vector<Record *> &Features) {
RISCVISAUtils::OrderedExtensionMap Extensions;
unsigned XLen = 0;
// Convert features to FeatureVector.
for (auto *Feature : Features) {
StringRef FeatureName = getExtensionName(Feature);
if (Feature->isSubClassOf("RISCVExtension")) {
unsigned Major = Feature->getValueAsInt("MajorVersion");
unsigned Minor = Feature->getValueAsInt("MinorVersion");
Extensions[FeatureName.str()] = {Major, Minor};
} else if (FeatureName == "64bit") {
assert(XLen == 0 && "Already determined XLen");
XLen = 64;
} else if (FeatureName == "32bit") {
assert(XLen == 0 && "Already determined XLen");
XLen = 32;
}
}
assert(XLen != 0 && "Unable to determine XLen");
OS << "rv" << XLen;
ListSeparator LS("_");
for (auto const &Ext : Extensions)
OS << LS << Ext.first << Ext.second.Major << 'p' << Ext.second.Minor;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wangpc-pp @topperc Are there any equivalents of the helper printMArch?

No, I think there isn't. You may need to write a helper via RISCVISAInfo::parseFeatures and RISCVISAInfo::toString().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See

void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args,
std::vector<StringRef> &Features) {
StringRef MArch = getRISCVArch(Args, Triple);
if (!getArchFeatures(D, MArch, Features, Args))
return;
.

Copy link

github-actions bot commented Jul 7, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Jul 13, 2024

I will rebase on the top of #97824.

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Jul 13, 2024

dtcxzyw@bananapif3:/data/llvm-build$ bin/clang -mcpu=native --print-enabled-extensions
clang version 19.0.0git
Target: riscv64-unknown-linux-gnu
Thread model: posix
InstalledDir: /data/llvm-build/bin
Build config: +assertions
Extensions enabled for the given RISC-V target

    Name                 Version   Description
    i                    2.1       'I' (Base Integer Instruction Set)
    m                    2.0       'M' (Integer Multiplication and Division)
    a                    2.1       'A' (Atomic Instructions)
    f                    2.2       'F' (Single-Precision Floating-Point)
    d                    2.2       'D' (Double-Precision Floating-Point)
    c                    2.0       'C' (Compressed Instructions)
    v                    1.0       'V' (Vector Extension for Application Processors)
    zicond               1.0       'Zicond' (Integer Conditional Operations)
    zicsr                2.0       'zicsr' (CSRs)
    zihintpause          2.0       'Zihintpause' (Pause Hint)
    zmmul                1.0       'Zmmul' (Integer Multiplication)
    zba                  1.0       'Zba' (Address Generation Instructions)
    zbb                  1.0       'Zbb' (Basic Bit-Manipulation)
    zbc                  1.0       'Zbc' (Carry-Less Multiplication)
    zbs                  1.0       'Zbs' (Single-Bit Instructions)
    zve32f               1.0       'Zve32f' (Vector Extensions for Embedded Processors with maximal 32 EEW and F extension)
    zve32x               1.0       'Zve32x' (Vector Extensions for Embedded Processors with maximal 32 EEW)
    zve64d               1.0       'Zve64d' (Vector Extensions for Embedded Processors with maximal 64 EEW, F and D extension)
    zve64f               1.0       'Zve64f' (Vector Extensions for Embedded Processors with maximal 64 EEW and F extension)
    zve64x               1.0       'Zve64x' (Vector Extensions for Embedded Processors with maximal 64 EEW)
    zvl128b              1.0       'Zvl' (Minimum Vector Length) 128
    zvl32b               1.0       'Zvl' (Minimum Vector Length) 32
    zvl64b               1.0       'Zvl' (Minimum Vector Length) 64

Experimental extensions

ISA String: rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_v1p0_zicond1p0_zicsr2p0_zihintpause2p0_zmmul1p0_zba1p0_zbb1p0_zbc1p0_zbs1p0_zve32f1p0_zve32x1p0_zve64d1p0_zve64f1p0_zve64x1p0_zvl128b1p0_zvl32b1p0_zvl64b1p0
dtcxzyw@bananapif3:/data/llvm-build$

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Jul 13, 2024

Ping @wangpc-pp @topperc @preames

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants