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][FMV] Support target_clones #85786

Merged
merged 18 commits into from
Sep 13, 2024
Merged

Conversation

BeMg
Copy link
Contributor

@BeMg BeMg commented Mar 19, 2024

This patch enable the function multiversion(FMV) and target_clones attribute for RISC-V target.

The proposal of target_clones syntax can be found at the riscv-non-isa/riscv-c-api-doc#48 (which has landed), as modified by the proposed riscv-non-isa/riscv-c-api-doc#85 (which adds the priority syntax).

It supports the target_clones function attribute and function multiversioning feature for RISC-V target. It will generate the ifunc resolver function for the function that declared with target_clones attribute.

The resolver function will check the version support by runtime object __riscv_feature_bits.

For example:

__attribute__((target_clones("default", "arch=+ver1", "arch=+ver2"))) int bar() {
    return 1;
}

the corresponding resolver will be like:

bar.resolver() {
    __init_riscv_feature_bits();
    // Check arch=+ver1
    if ((__riscv_feature_bits.features[0] & BITMASK_OF_VERSION1) == BITMASK_OF_VERSION1) {
        return bar.arch=+ver1;
    } else {
        // Check arch=+ver2
        if ((__riscv_feature_bits.features[0] & BITMASK_OF_VERSION2) == BITMASK_OF_VERSION2) {
            return bar.arch=+ver2;
        } else {
            // Default
            return bar.default;
        }
    }
}

@BeMg
Copy link
Contributor Author

BeMg commented Mar 19, 2024

Inside the resolver function, a function named __riscv_ifunc_select is used to ensure that all additional extension requirements are met. It is temporary interface until proposal be ratified.

bool __riscv_ifunc_select(char *FeatureStr);

@BeMg
Copy link
Contributor Author

BeMg commented Mar 19, 2024

The candidate function priority order is undefined. Currently, it base on declaration order.

@BeMg
Copy link
Contributor Author

BeMg commented Mar 19, 2024

The warning occurs because the RISC-V target_clones syntax __attribute__((target_clones("default", "arch=+zba,+v,+c,+zicond", "arch=+zbb,+c,+v"))) contains a comma inside double quotes.

warning: mixing 'target_clones' specifier mechanisms is permitted for GCC compatibility; 
use a comma separated sequence of string literals, or a string literal containing a comma-separated list of versions

Should we suppress this warning for RISC-V target? Or change the target_clones syntax?

@BeMg BeMg marked this pull request as ready for review March 21, 2024 06:52
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Mar 21, 2024
@BeMg BeMg requested review from kito-cheng and topperc March 21, 2024 06:52
@llvmbot
Copy link
Member

llvmbot commented Mar 21, 2024

@llvm/pr-subscribers-llvm-support
@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang

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

Author: Piyou Chen (BeMg)

Changes

This patch enable the function multiversion(FMV) and target_clones attribute for RISC-V target.

It will emit the IFUNC resolver function to select appropriate function during runtime.


Patch is 31.07 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/85786.diff

12 Files Affected:

  • (modified) clang/include/clang/Basic/TargetInfo.h (+2-1)
  • (modified) clang/lib/AST/ASTContext.cpp (+9)
  • (modified) clang/lib/Basic/Targets/RISCV.cpp (+8-2)
  • (modified) clang/lib/Basic/Targets/RISCV.h (+2)
  • (modified) clang/lib/CodeGen/CodeGenFunction.cpp (+101-1)
  • (modified) clang/lib/CodeGen/CodeGenFunction.h (+3)
  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+2)
  • (modified) clang/lib/CodeGen/Targets/RISCV.cpp (+23)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (+22)
  • (added) clang/test/CodeGen/attr-target-clones-riscv.c (+135)
  • (added) clang/test/CodeGenCXX/attr-target-clones-riscv.cpp (+136)
  • (added) clang/test/SemaCXX/attr-target-clones-riscv.cpp (+28)
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 374595edd2ce4a..aa48596fbce07d 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1445,7 +1445,8 @@ class TargetInfo : public TransferrableTargetInfo,
   /// Identify whether this target supports multiversioning of functions,
   /// which requires support for cpu_supports and cpu_is functionality.
   bool supportsMultiVersioning() const {
-    return getTriple().isX86() || getTriple().isAArch64();
+    return getTriple().isX86() || getTriple().isAArch64() ||
+           getTriple().isRISCV();
   }
 
   /// Identify whether this target supports IFuncs.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 5a8fae76a43a4d..0fd75e0b36b123 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -13636,6 +13636,15 @@ void ASTContext::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
       Features.insert(Features.begin(),
                       Target->getTargetOpts().FeaturesAsWritten.begin(),
                       Target->getTargetOpts().FeaturesAsWritten.end());
+    } else if (Target->getTriple().isRISCV()) {
+      if (VersionStr != "default") {
+        ParsedTargetAttr ParsedAttr = Target->parseTargetAttr(VersionStr);
+        Features.insert(Features.begin(), ParsedAttr.Features.begin(),
+                        ParsedAttr.Features.end());
+      }
+      Features.insert(Features.begin(),
+                      Target->getTargetOpts().FeaturesAsWritten.begin(),
+                      Target->getTargetOpts().FeaturesAsWritten.end());
     } else {
       if (VersionStr.starts_with("arch="))
         TargetCPU = VersionStr.drop_front(sizeof("arch=") - 1);
diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index a6d4af2b88111a..8e9132c9191a3c 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -257,7 +257,7 @@ bool RISCVTargetInfo::initFeatureMap(
 
   // If a target attribute specified a full arch string, override all the ISA
   // extension target features.
-  const auto I = llvm::find(FeaturesVec, "__RISCV_TargetAttrNeedOverride");
+  const auto I = llvm::find(FeaturesVec, "+__RISCV_TargetAttrNeedOverride");
   if (I != FeaturesVec.end()) {
     std::vector<std::string> OverrideFeatures(std::next(I), FeaturesVec.end());
 
@@ -366,6 +366,12 @@ bool RISCVTargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
   return true;
 }
 
+bool RISCVTargetInfo::isValidFeatureName(StringRef Feature) const {
+  if (Feature.starts_with("__RISCV_TargetAttrNeedOverride"))
+    return true;
+  return llvm::RISCVISAInfo::isSupportedExtensionFeature(Feature);
+}
+
 bool RISCVTargetInfo::isValidCPUName(StringRef Name) const {
   bool Is64Bit = getTriple().isArch64Bit();
   return llvm::RISCV::parseCPU(Name, Is64Bit);
@@ -390,7 +396,7 @@ void RISCVTargetInfo::fillValidTuneCPUList(
 
 static void handleFullArchString(StringRef FullArchStr,
                                  std::vector<std::string> &Features) {
-  Features.push_back("__RISCV_TargetAttrNeedOverride");
+  Features.push_back("+__RISCV_TargetAttrNeedOverride");
   auto RII = llvm::RISCVISAInfo::parseArchString(
       FullArchStr, /* EnableExperimentalExtension */ true);
   if (llvm::errorToBool(RII.takeError())) {
diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index bfbdafb682c851..ef8f59185d753c 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/clang/lib/Basic/Targets/RISCV.h
@@ -106,6 +106,8 @@ class RISCVTargetInfo : public TargetInfo {
   bool handleTargetFeatures(std::vector<std::string> &Features,
                             DiagnosticsEngine &Diags) override;
 
+  bool isValidFeatureName(StringRef Feature) const override;
+
   bool hasBitIntType() const override { return true; }
 
   bool hasBFloat16Type() const override { return true; }
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 4a3ff49b0007a3..544c696376cd89 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2803,12 +2803,112 @@ void CodeGenFunction::EmitMultiVersionResolver(
   case llvm::Triple::aarch64:
     EmitAArch64MultiVersionResolver(Resolver, Options);
     return;
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
+    EmitRISCVMultiVersionResolver(Resolver, Options);
+    return;
 
   default:
-    assert(false && "Only implemented for x86 and AArch64 targets");
+    assert(false && "Only implemented for x86, AArch64 and RISC-V targets");
   }
 }
 
+void CodeGenFunction::EmitRISCVMultiVersionResolver(
+    llvm::Function *Resolver, ArrayRef<MultiVersionResolverOption> Options) {
+
+  auto EmitIFUNCFeatureCheckFunc =
+      [&](std::string CurrFeatStr) -> llvm::Value * {
+    llvm::Constant *FeatStr = Builder.CreateGlobalString(CurrFeatStr);
+    llvm::Type *Args[] = {Int8PtrTy};
+    llvm::FunctionType *FTy =
+        llvm::FunctionType::get(Builder.getInt1Ty(), Args, /*Variadic*/ false);
+    llvm::FunctionCallee Func =
+        CGM.CreateRuntimeFunction(FTy, "__riscv_ifunc_select");
+    cast<llvm::GlobalValue>(Func.getCallee())->setDSOLocal(true);
+    cast<llvm::GlobalValue>(Func.getCallee())
+        ->setDLLStorageClass(llvm::GlobalValue::DefaultStorageClass);
+    return Builder.CreateCall(Func, FeatStr);
+  };
+
+  llvm::BasicBlock *CurBlock = createBasicBlock("resolver_entry", Resolver);
+  Builder.SetInsertPoint(CurBlock);
+
+  bool SupportsIFunc = getContext().getTargetInfo().supportsIFunc();
+  bool HasDefault = false;
+  int DefaultIndex = 0;
+  // Check the each candidate function.
+  for (unsigned Index = 0; Index < Options.size(); Index++) {
+
+    if (Options[Index].Conditions.Features[0].starts_with("default")) {
+      HasDefault = true;
+      DefaultIndex = Index;
+      continue;
+    }
+
+    Builder.SetInsertPoint(CurBlock);
+
+    std::vector<std::string> TargetAttrFeats =
+        getContext()
+            .getTargetInfo()
+            .parseTargetAttr(Options[Index].Conditions.Features[0])
+            .Features;
+
+    if (!TargetAttrFeats.empty()) {
+      // If this function doens't need override, then merge with module level
+      // target features. Otherwise, remain the current target features.
+      auto I = llvm::find(TargetAttrFeats, "+__RISCV_TargetAttrNeedOverride");
+      if (I == TargetAttrFeats.end())
+        TargetAttrFeats.insert(TargetAttrFeats.begin(),
+                               Target.getTargetOpts().FeaturesAsWritten.begin(),
+                               Target.getTargetOpts().FeaturesAsWritten.end());
+      else
+        TargetAttrFeats.erase(I);
+
+      // Only consider +<extension-feature>.
+      std::vector<std::string> PlusTargetAttrFeats;
+      for (StringRef Feat : TargetAttrFeats) {
+        if (!getContext().getTargetInfo().isValidFeatureName(
+                Feat.substr(1).str()))
+          continue;
+        if (Feat.starts_with("+"))
+          PlusTargetAttrFeats.push_back(Feat.substr(1).str());
+      }
+
+      // Join with ';' delimiter
+      std::string CurrFeatStr = std::accumulate(
+          std::begin(PlusTargetAttrFeats), std::end(PlusTargetAttrFeats),
+          std::string(), [](const std::string &a, const std::string &b) {
+            return a.empty() ? b : a + ";" + b;
+          });
+
+      llvm::Value *Condition = EmitIFUNCFeatureCheckFunc(CurrFeatStr);
+      llvm::BasicBlock *RetBlock =
+          createBasicBlock("resolver_return", Resolver);
+      CGBuilderTy RetBuilder(*this, RetBlock);
+      CreateMultiVersionResolverReturn(CGM, Resolver, RetBuilder,
+                                       Options[Index].Function, SupportsIFunc);
+      CurBlock = createBasicBlock("resolver_else", Resolver);
+      Builder.CreateCondBr(Condition, RetBlock, CurBlock);
+    }
+  }
+
+  // Finally, emit the default one.
+  if (HasDefault) {
+    Builder.SetInsertPoint(CurBlock);
+    CreateMultiVersionResolverReturn(
+        CGM, Resolver, Builder, Options[DefaultIndex].Function, SupportsIFunc);
+    return;
+  }
+
+  // If no generic/default, emit an unreachable.
+  Builder.SetInsertPoint(CurBlock);
+  llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap);
+  TrapCall->setDoesNotReturn();
+  TrapCall->setDoesNotThrow();
+  Builder.CreateUnreachable();
+  Builder.ClearInsertionPoint();
+}
+
 void CodeGenFunction::EmitAArch64MultiVersionResolver(
     llvm::Function *Resolver, ArrayRef<MultiVersionResolverOption> Options) {
   assert(!Options.empty() && "No multiversion resolver options found");
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index e8f8aa601ed017..e62657de7b7323 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -5013,6 +5013,9 @@ class CodeGenFunction : public CodeGenTypeCache {
   void
   EmitAArch64MultiVersionResolver(llvm::Function *Resolver,
                                   ArrayRef<MultiVersionResolverOption> Options);
+  void
+  EmitRISCVMultiVersionResolver(llvm::Function *Resolver,
+                                ArrayRef<MultiVersionResolverOption> Options);
 
 private:
   QualType getVarArgType(const Expr *Arg);
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 8ceecff28cbc63..e571dbb384b78d 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4161,6 +4161,8 @@ void CodeGenModule::emitMultiVersionFunctions() {
             for (auto &CurFeat : VerFeats)
               Feature.push_back(CurFeat.trim());
           }
+        } else if (getTarget().getTriple().isRISCV()) {
+          Feature.push_back(Version);
         } else {
           if (Version.starts_with("arch="))
             Architecture = Version.drop_front(sizeof("arch=") - 1);
diff --git a/clang/lib/CodeGen/Targets/RISCV.cpp b/clang/lib/CodeGen/Targets/RISCV.cpp
index 9a79424c4612ce..30c10c35133161 100644
--- a/clang/lib/CodeGen/Targets/RISCV.cpp
+++ b/clang/lib/CodeGen/Targets/RISCV.cpp
@@ -63,9 +63,32 @@ class RISCVABIInfo : public DefaultABIInfo {
                                                CharUnits Field2Off) const;
 
   ABIArgInfo coerceVLSVector(QualType Ty) const;
+
+  using ABIInfo::appendAttributeMangling;
+  void appendAttributeMangling(TargetClonesAttr *Attr, unsigned Index,
+                               raw_ostream &Out) const override;
+  void appendAttributeMangling(StringRef AttrStr,
+                               raw_ostream &Out) const override;
 };
 } // end anonymous namespace
 
+void RISCVABIInfo::appendAttributeMangling(TargetClonesAttr *Attr,
+                                           unsigned Index,
+                                           raw_ostream &Out) const {
+  appendAttributeMangling(Attr->getFeatureStr(Index), Out);
+}
+
+void RISCVABIInfo::appendAttributeMangling(StringRef AttrStr,
+                                           raw_ostream &Out) const {
+  if (AttrStr == "default") {
+    Out << ".default";
+    return;
+  }
+
+  Out << '.';
+  Out << AttrStr;
+}
+
 void RISCVABIInfo::computeInfo(CGFunctionInfo &FI) const {
   QualType RetTy = FI.getReturnType();
   if (!getCXXABI().classifyReturnType(FI))
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index ec00fdf3f88d9e..9e44e6a3b70734 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3626,6 +3626,28 @@ bool Sema::checkTargetClonesAttrString(
       if (hasArmStreamingInterface(cast<FunctionDecl>(D)))
         return Diag(LiteralLoc,
                     diag::err_sme_streaming_cannot_be_multiversioned);
+    } else if (TInfo.getTriple().isRISCV()) {
+      if (Str.starts_with("arch=")) {
+        // parseTargetAttr will parse full version string,
+        // the following split Cur string is no longer interesting.
+        if ((!Cur.starts_with("arch=")))
+          continue;
+
+        ParsedTargetAttr TargetAttr =
+            Context.getTargetInfo().parseTargetAttr(Str);
+        if (TargetAttr.Features.empty())
+          return Diag(CurLoc, diag::warn_unsupported_target_attribute)
+                 << Unsupported << None << Str << TargetClones;
+      } else if (Str == "default") {
+        DefaultIsDupe = HasDefault;
+        HasDefault = true;
+      } else {
+        return Diag(CurLoc, diag::warn_unsupported_target_attribute)
+               << Unsupported << None << Str << TargetClones;
+      }
+      if (llvm::is_contained(StringsBuffer, Str) || DefaultIsDupe)
+        Diag(CurLoc, diag::warn_target_clone_duplicate_options);
+      StringsBuffer.push_back(Str);
     } else {
       // Other targets ( currently X86 )
       if (Cur.starts_with("arch=")) {
diff --git a/clang/test/CodeGen/attr-target-clones-riscv.c b/clang/test/CodeGen/attr-target-clones-riscv.c
new file mode 100644
index 00000000000000..70b74befb54ffb
--- /dev/null
+++ b/clang/test/CodeGen/attr-target-clones-riscv.c
@@ -0,0 +1,135 @@
+// RUN: %clang_cc1 -triple riscv64-linux-gnu -target-feature +i -S -emit-llvm -o - %s | FileCheck %s
+
+__attribute__((target_clones("default", "arch=rv64gc"))) int foo1(void) { return 1; }
+__attribute__((target_clones("default", "arch=+zbb"))) int foo2(void) { return 2; }
+__attribute__((target_clones("default", "arch=+zbb,+c"))) int foo3(void) { return 3; }
+__attribute__((target_clones("default", "arch=rv64gc", "arch=+zbb,+v"))) int foo4(void) { return 4; }
+__attribute__((target_clones("default"))) int foo5(void) { return 5; }
+
+int bar() {
+  return foo1() + foo2() + foo3() + foo4();
+}
+
+//.
+// CHECK: @[[GLOB0:[0-9]+]] = private unnamed_addr constant [25 x i8] c"m;a;f;d;c;zicsr;zifencei\00", align 1
+// CHECK: @[[GLOB1:[0-9]+]] = private unnamed_addr constant [6 x i8] c"i;zbb\00", align 1
+// CHECK: @[[GLOB2:[0-9]+]] = private unnamed_addr constant [8 x i8] c"i;zbb;c\00", align 1
+// CHECK: @[[GLOB3:[0-9]+]] = private unnamed_addr constant [25 x i8] c"m;a;f;d;c;zicsr;zifencei\00", align 1
+// CHECK: @[[GLOB4:[0-9]+]] = private unnamed_addr constant [8 x i8] c"i;zbb;v\00", align 1
+// CHECK: @foo1.ifunc = weak_odr alias i32 (), ptr @foo1
+// CHECK: @foo2.ifunc = weak_odr alias i32 (), ptr @foo2
+// CHECK: @foo3.ifunc = weak_odr alias i32 (), ptr @foo3
+// CHECK: @foo4.ifunc = weak_odr alias i32 (), ptr @foo4
+// CHECK: @foo5.ifunc = weak_odr alias i32 (), ptr @foo5
+// CHECK: @foo1 = weak_odr ifunc i32 (), ptr @foo1.resolver
+// CHECK: @foo2 = weak_odr ifunc i32 (), ptr @foo2.resolver
+// CHECK: @foo3 = weak_odr ifunc i32 (), ptr @foo3.resolver
+// CHECK: @foo4 = weak_odr ifunc i32 (), ptr @foo4.resolver
+// CHECK: @foo5 = weak_odr ifunc i32 (), ptr @foo5.resolver
+//.
+// CHECK-LABEL: define dso_local signext i32 @foo1.default(
+// CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    ret i32 1
+//
+//
+// CHECK-LABEL: define weak_odr ptr @foo1.resolver() comdat {
+// CHECK-NEXT:  resolver_entry:
+// CHECK-NEXT:    [[TMP0:%.*]] = call i1 @__riscv_ifunc_select(ptr @[[GLOB0]])
+// CHECK-NEXT:    br i1 [[TMP0]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK:       resolver_return:
+// CHECK-NEXT:    ret ptr @"foo1.arch=rv64gc"
+// CHECK:       resolver_else:
+// CHECK-NEXT:    ret ptr @foo1.default
+//
+//
+// CHECK-LABEL: define dso_local signext i32 @foo2.default(
+// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    ret i32 2
+//
+//
+// CHECK-LABEL: define weak_odr ptr @foo2.resolver() comdat {
+// CHECK-NEXT:  resolver_entry:
+// CHECK-NEXT:    [[TMP0:%.*]] = call i1 @__riscv_ifunc_select(ptr @[[GLOB1]])
+// CHECK-NEXT:    br i1 [[TMP0]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK:       resolver_return:
+// CHECK-NEXT:    ret ptr @"foo2.arch=+zbb"
+// CHECK:       resolver_else:
+// CHECK-NEXT:    ret ptr @foo2.default
+//
+//
+// CHECK-LABEL: define dso_local signext i32 @foo3.default(
+// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    ret i32 3
+//
+//
+// CHECK-LABEL: define weak_odr ptr @foo3.resolver() comdat {
+// CHECK-NEXT:  resolver_entry:
+// CHECK-NEXT:    [[TMP0:%.*]] = call i1 @__riscv_ifunc_select(ptr @[[GLOB2]])
+// CHECK-NEXT:    br i1 [[TMP0]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK:       resolver_return:
+// CHECK-NEXT:    ret ptr @"foo3.arch=+zbb,+c"
+// CHECK:       resolver_else:
+// CHECK-NEXT:    ret ptr @foo3.default
+//
+//
+// CHECK-LABEL: define dso_local signext i32 @foo4.default(
+// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    ret i32 4
+//
+//
+// CHECK-LABEL: define weak_odr ptr @foo4.resolver() comdat {
+// CHECK-NEXT:  resolver_entry:
+// CHECK-NEXT:    [[TMP0:%.*]] = call i1 @__riscv_ifunc_select(ptr @[[GLOB3]])
+// CHECK-NEXT:    br i1 [[TMP0]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK:       resolver_return:
+// CHECK-NEXT:    ret ptr @"foo4.arch=rv64gc"
+// CHECK:       resolver_else:
+// CHECK-NEXT:    [[TMP1:%.*]] = call i1 @__riscv_ifunc_select(ptr @[[GLOB4]])
+// CHECK-NEXT:    br i1 [[TMP1]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2:%.*]]
+// CHECK:       resolver_return1:
+// CHECK-NEXT:    ret ptr @"foo4.arch=+zbb,+v"
+// CHECK:       resolver_else2:
+// CHECK-NEXT:    ret ptr @foo4.default
+//
+//
+// CHECK-LABEL: define dso_local signext i32 @foo5.default(
+// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    ret i32 5
+//
+//
+// CHECK-LABEL: define weak_odr ptr @foo5.resolver() comdat {
+// CHECK-NEXT:  resolver_entry:
+// CHECK-NEXT:    ret ptr @foo5.default
+//
+//
+// CHECK-LABEL: define dso_local signext i32 @bar(
+// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[CALL:%.*]] = call signext i32 @foo1()
+// CHECK-NEXT:    [[CALL1:%.*]] = call signext i32 @foo2()
+// CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[CALL]], [[CALL1]]
+// CHECK-NEXT:    [[CALL2:%.*]] = call signext i32 @foo3()
+// CHECK-NEXT:    [[ADD3:%.*]] = add nsw i32 [[ADD]], [[CALL2]]
+// CHECK-NEXT:    [[CALL4:%.*]] = call signext i32 @foo4()
+// CHECK-NEXT:    [[ADD5:%.*]] = add nsw i32 [[ADD3]], [[CALL4]]
+// CHECK-NEXT:    ret i32 [[ADD5]]
+//
+//.
+// CHECK: attributes #[[ATTR0]] = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+64bit,+i" }
+// CHECK: attributes #[[ATTR1:[0-9]+]] = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+64bit,+a,+c,+d,+f,+m,+zicsr,+zifencei,-e,-experimental-smmpm,-experimental-smnpm,-experimental-ssnpm,-experimental-sspm,-experimental-ssqosid,-experimental-supm,-experimental-zaamo,-experimental-zabha,-experimental-zalasr,-experimental-zalrsc,-experimental-zcmop,-experimental-zfbfmin,-experimental-zicfilp,-experimental-zicfiss,-experimental-zimop,-experimental-ztso,-experimental-zvfbfmin,-experimental-zvfbfwma,-h,-shcounterenw,-shgatpa,-shtvala,-shvsatpa,-shvstvala,-shvstvecd,-smaia,-smepmp,-ssaia,-ssccptr,-sscofpmf,-sscounterenw,-ssstateen,-ssstrict,-sstc,-sstvala,-sstvecd,-ssu64xl,-svade,-svadu,-svbare,-svinval,-svnapot,-svpbmt,-v,-xcvalu,-xcvbi,-xcvbitmanip,-xcvelw,-xcvmac,-xcvmem,-xcvsimd,-xsfcease,-xsfvcp,-xsfvfnrclipxfqf,-xsfvfwmaccqqq,-xsfvqmaccdod,-xsfvqmaccqoq,-xsifivecdiscarddlone,-xsifivecflushdlone,-xtheadba,-xtheadbb,-xtheadbs,-xtheadcmo,-xtheadcondmov,-xtheadfmemidx,-xtheadmac,-xtheadmemidx,-xtheadmempair,-xtheadsync,-xtheadvdot,-xventanacondops,-za128rs,-za64rs,-zacas,-zawrs,-zba,-zbb,-zbc,-zbkb,-zbkc,-zbkx,-zbs,-zca,-zcb,-zcd,-zce,-zcf,-zcmp,-zcmt,-zdinx,-zfa,-zfh,-zfhmin,-zfinx,-zhinx,-zhinxmin,-zic64b,-zicbom,-zicbop,-zicboz,-ziccamoa,-ziccif,-zicclsm,-ziccrse,-zicntr,-zicond,-zihintntl,-zihintpause,-zihpm,-zk,-zkn,-zknd,-zkne,-zknh,-zkr,-zks,-zksed,-zksh,-zkt,-zmmul,-zvbb,-zvbc,-zve32f,-zve32x,-zve64d,-zve64f,-zve64x,-zvfh,-zvfhmin,-zvkb,-zvkg,-zvkn,-zvknc,-zvkned,-zvkng,-zvknha,-zvknhb,-zvks,-zvksc,-zvksed,-zvksg,-zvksh,-zvkt,-zvl1024b,-zvl128b,-zvl16384b,-zvl2048b,-zvl256b,-zvl32768b,-zvl32b,-zvl4096b,-zvl512b,-zvl64b,-zvl65536b,-zvl8192b" }
+// CHECK: attributes #[[ATTR2:[0-9]+]] = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+64bit,+i,+zbb" }
+// CHECK: attributes #[[ATTR3:[0-9]+]] = { noinline nounwind optnone "n...
[truncated]

@BeMg BeMg requested a review from lukel97 March 21, 2024 06:52
@BeMg
Copy link
Contributor Author

BeMg commented Mar 21, 2024

The proposal can be found at the riscv-non-isa/riscv-c-api-doc#48.

@lukel97
Copy link
Contributor

lukel97 commented Mar 21, 2024

Can this land until #85790 lands?

@BeMg
Copy link
Contributor Author

BeMg commented Mar 30, 2024

  1. Suppress the warning warn_target_clone_mixed_values for RISC-V
  2. Update __riscv_ifunc_select. From __riscv_ifunc_select(char *) into __riscv_ifunc_select(unsigned long long, unsigned long long ).
  3. Add one more error messenge when there are +extension that hwprobe can't detect.

clang/lib/CodeGen/CGBuiltin.cpp Outdated Show resolved Hide resolved
clang/lib/CodeGen/CGBuiltin.cpp Outdated Show resolved Hide resolved
clang/lib/CodeGen/CGBuiltin.cpp Outdated Show resolved Hide resolved
clang/lib/CodeGen/CGBuiltin.cpp Outdated Show resolved Hide resolved
@BeMg BeMg force-pushed the IFUNC/FMV-target-clones branch from 1767d49 to ad2c9f3 Compare June 11, 2024 14:30
@BeMg
Copy link
Contributor Author

BeMg commented Jun 11, 2024

Stack on #94440

@BeMg BeMg force-pushed the IFUNC/FMV-target-clones branch from ad2c9f3 to aaa7017 Compare June 28, 2024 07:32
BeMg added 7 commits August 22, 2024 21:20

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@BeMg BeMg requested a review from topperc August 23, 2024 07:08
@BeMg
Copy link
Contributor Author

BeMg commented Aug 26, 2024

ping

@kito-cheng
Copy link
Member

A test case will crash, missing + before zbc:

__attribute__((target_clones("default", "arch=+zbb,zbc;priority=-1", "priority=-2;arch=+zba", "priority=3;arch=+zbb,+zba"))) int foo1(void) { return 2; }


int bar() { return foo1(); }
$ clang -cc1 -triple riscv64-linux-gnu -target-feature +i -emit-llvm foo.c
clang: /home/kitoc/llvm-workspace/llvm-project/llvm/lib/TargetParser/RISCVISAInfo.cpp:455: static llvm::Expected<std::unique_ptr<llvm::RISCVISAInfo> > llvm::RISCVISAInfo::parseFeatures(unsigned int, const std::vector<std::__cxx11::basic_string<char> >&): Assertion `ExtName.size() > 1 && (ExtName[0] == '+' || ExtName[0] == '-')' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: ./bin/clang -cc1 -triple riscv64-linux-gnu -target-feature +i -emit-llvm attr-target-clones-riscv.c
1.      attr-target-clones-riscv.c:7:1: current parser token 'int'
2.      attr-target-clones-riscv.c:4:130: LLVM IR generation of declaration 'foo1'
3.      attr-target-clones-riscv.c:4:130: Generating code for declaration 'foo1'
 #0 0x00007f4ed63e00d0 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/scratch/kitoc/llvm-workspace/build-upstream/bin/../lib/libLLVMSupport.so.20.0git+0x1e00d0)
...
#11 0x00007f4eda71a854 llvm::RISCVISAInfo::parseFeatures(unsigned int, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&) (/scratch/kitoc/llvm-workspace/build-upstream/bin/../lib/libLLVMTargetParser.so.20.0git+0x3d854)
#12 0x00007f4ed825b8e0 clang::targets::RISCVTargetInfo::initFeatureMap(llvm::StringMap<bool, llvm::MallocAllocator>&, clang::DiagnosticsEngine&, llvm::StringRef, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&) const (/scratch/kitoc/llvm-workspace/build-upstream/bin/../lib/libclangBasic.so.20.0git+0x45b8e0)
...
Aborted (core dumped)

@BeMg
Copy link
Contributor Author

BeMg commented Sep 6, 2024

A test case will crash, missing + before zbc:

__attribute__((target_clones("default", "arch=+zbb,zbc;priority=-1", "priority=-2;arch=+zba", "priority=3;arch=+zbb,+zba"))) int foo1(void) { return 2; }


int bar() { return foo1(); }
$ clang -cc1 -triple riscv64-linux-gnu -target-feature +i -emit-llvm foo.c
clang: /home/kitoc/llvm-workspace/llvm-project/llvm/lib/TargetParser/RISCVISAInfo.cpp:455: static llvm::Expected<std::unique_ptr<llvm::RISCVISAInfo> > llvm::RISCVISAInfo::parseFeatures(unsigned int, const std::vector<std::__cxx11::basic_string<char> >&): Assertion `ExtName.size() > 1 && (ExtName[0] == '+' || ExtName[0] == '-')' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: ./bin/clang -cc1 -triple riscv64-linux-gnu -target-feature +i -emit-llvm attr-target-clones-riscv.c
1.      attr-target-clones-riscv.c:7:1: current parser token 'int'
2.      attr-target-clones-riscv.c:4:130: LLVM IR generation of declaration 'foo1'
3.      attr-target-clones-riscv.c:4:130: Generating code for declaration 'foo1'
 #0 0x00007f4ed63e00d0 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/scratch/kitoc/llvm-workspace/build-upstream/bin/../lib/libLLVMSupport.so.20.0git+0x1e00d0)
...
#11 0x00007f4eda71a854 llvm::RISCVISAInfo::parseFeatures(unsigned int, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&) (/scratch/kitoc/llvm-workspace/build-upstream/bin/../lib/libLLVMTargetParser.so.20.0git+0x3d854)
#12 0x00007f4ed825b8e0 clang::targets::RISCVTargetInfo::initFeatureMap(llvm::StringMap<bool, llvm::MallocAllocator>&, clang::DiagnosticsEngine&, llvm::StringRef, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&) const (/scratch/kitoc/llvm-workspace/build-upstream/bin/../lib/libclangBasic.so.20.0git+0x45b8e0)
...
Aborted (core dumped)

Fixed and add new test.

@BeMg
Copy link
Contributor Author

BeMg commented Sep 6, 2024

Rename and update the description of err_os_unsupport_riscv_target_clones because not only target clones trigger FMV.

@BeMg BeMg requested a review from kito-cheng September 6, 2024 13:33
Copy link
Member

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

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

LGTM, and I would prefer wait one more LGTM from Craig or Philip OR wait one more week to make sure no further comment :)

clang/include/clang/Basic/DiagnosticFrontendKinds.td Outdated Show resolved Hide resolved
clang/lib/CodeGen/Targets/RISCV.cpp Outdated Show resolved Hide resolved
clang/lib/CodeGen/Targets/RISCV.cpp Outdated Show resolved Hide resolved
clang/lib/CodeGen/CodeGenFunction.cpp Outdated Show resolved Hide resolved
clang/lib/CodeGen/CodeGenFunction.cpp Outdated Show resolved Hide resolved
@BeMg BeMg requested a review from topperc September 12, 2024 15:15
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

__attribute__((target_clones("default", "arch=+zbb", "arch=+zba", "arch=+zbb,+zba"))) int foo7(void) { return 2; }
__attribute__((target_clones("default", "arch=+zbb;priority=2", "arch=+zba;priority=1", "arch=+zbb,+zba;priority=3"))) int foo8(void) { return 2; }
__attribute__((target_clones("default", "arch=+zbb;priority=1", "priority=2;arch=+zba", "priority=3;arch=+zbb,+zba"))) int foo9(void) { return 2; }
__attribute__((target_clones("default", "arch=+zbb;priority=-1", "priority=-2;arch=+zba", "priority=3;arch=+zbb,+zba"))) int foo10(void) { return 2; }
Copy link
Member

Choose a reason for hiding this comment

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

The grammar defined in riscv-non-isa/riscv-c-api-doc#85 does not allow for priority=-1, as you're only allowed the digits 0-9 after priority=. Was it intentional to support or not support negative priorities?

Copy link
Contributor Author

@BeMg BeMg Sep 13, 2024

Choose a reason for hiding this comment

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

Thank you for pointing that out. I've updated the grammar to support negative priorities.

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

(I only did a quick pass on this version assuming mostly unchanged from prior. Wanted to explicitly chime in with support for the priority syntax.)

@BeMg BeMg merged commit 9cd9377 into llvm:main Sep 13, 2024
8 checks passed
BeMg added a commit that referenced this pull request Oct 4, 2024
This patch enable `target_version` attribute for RISC-V target.

The proposal of `target_version` syntax can be found at the
riscv-non-isa/riscv-c-api-doc#48 (which has
landed), as modified by the proposed
riscv-non-isa/riscv-c-api-doc#85 (which adds the
priority syntax).

`target_version` attribute will trigger the function multi-versioning
feature and act like `target_clones` attribute. See
#85786 for the implementation
of `target_clones`.
BeMg added a commit that referenced this pull request Oct 8, 2024
Fix the buildbot failure caused by heap use-after-free error.

Origin message:

    This patch enable `target_version` attribute for RISC-V target.

    The proposal of `target_version` syntax can be found at the
    riscv-non-isa/riscv-c-api-doc#48 (which has
    landed), as modified by the proposed
riscv-non-isa/riscv-c-api-doc#85 (which adds the
    priority syntax).

`target_version` attribute will trigger the function multi-versioning
    feature and act like `target_clones` attribute. See
#85786 for the implementation
    of `target_clones`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V clang:codegen clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category llvm:support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants