Skip to content

[FMV][RISCV] Add __riscv_feature_bits.length check #110098

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[FMV][RISCV] Add __riscv_feature_bits.length check #110098

wants to merge 2 commits into from

Conversation

BeMg
Copy link
Contributor

@BeMg BeMg commented Sep 26, 2024

This patch add a condition that check the __riscv_feature_bits.length before access __riscv_feature_bits.features.

It updates the resolver function as the following structure.

if (__riscv_feature_bits.features.length <=
    llvm::RISCVISAInfo::FeatureBitSize) {
    if (FeaturesConditionVersion1)
        return Version1;
    else if (FeaturesConditionVersion2)
        return Version2;
    else if (FeaturesConditionVersion3)
        return Version3;
    ...
}
return DefaultVersion;

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:codegen IR generation bugs: mangling, exceptions, etc. labels Sep 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 26, 2024 •
edited
Loading

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Piyou Chen (BeMg)

Changes

This patch add a condition that check the __riscv_feature_bits.length before access __riscv_feature_bits.features.

It updates the resolver function as the following structure.

if (__riscv_feature_bits.features.length &lt;=
    llvm::RISCVISAInfo::FeatureBitSize) {
    if (FeaturesConditionVersion1)
        return Version1;
    else if (FeaturesConditionVersion2)
        return Version2;
    else if (FeaturesConditionVersion3)
        return Version3;
    ...
}
return DefaultVersion;

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

5 Files Affected:

  • (modified) clang/lib/CodeGen/CGBuiltin.cpp (+26-7)
  • (modified) clang/lib/CodeGen/CodeGenFunction.cpp (+23-9)
  • (modified) clang/lib/CodeGen/CodeGenFunction.h (+1)
  • (modified) clang/test/CodeGen/attr-target-clones-riscv.c (+112-72)
  • (modified) clang/test/CodeGenCXX/attr-target-clones-riscv.cpp (+112-72)
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 249aead33ad73d..6af6f166575142 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -14534,8 +14534,9 @@ Value *CodeGenFunction::EmitRISCVCpuSupports(const CallExpr *E) {
   return EmitRISCVCpuSupports(ArrayRef<StringRef>(FeatureStr));
 }
 
-static Value *loadRISCVFeatureBits(unsigned Index, CGBuilderTy &Builder,
-                                   CodeGenModule &CGM) {
+static Value *loadRISCVFeatureBitsCommon(ArrayRef<llvm::Value *> GEPIndices,
+                                         CGBuilderTy &Builder,
+                                         CodeGenModule &CGM) {
   llvm::Type *Int32Ty = Builder.getInt32Ty();
   llvm::Type *Int64Ty = Builder.getInt64Ty();
   llvm::ArrayType *ArrayOfInt64Ty =
@@ -14544,14 +14545,32 @@ static Value *loadRISCVFeatureBits(unsigned Index, CGBuilderTy &Builder,
   llvm::Constant *RISCVFeaturesBits =
       CGM.CreateRuntimeVariable(StructTy, "__riscv_feature_bits");
   cast<llvm::GlobalValue>(RISCVFeaturesBits)->setDSOLocal(true);
-  Value *IndexVal = llvm::ConstantInt::get(Int32Ty, Index);
-  llvm::Value *GEPIndices[] = {Builder.getInt32(0), Builder.getInt32(1),
-                               IndexVal};
   Value *Ptr =
       Builder.CreateInBoundsGEP(StructTy, RISCVFeaturesBits, GEPIndices);
-  Value *FeaturesBit =
+  Value *FeaturesVal =
       Builder.CreateAlignedLoad(Int64Ty, Ptr, CharUnits::fromQuantity(8));
-  return FeaturesBit;
+  return FeaturesVal;
+}
+
+static Value *loadRISCVFeatureBitsLength(CGBuilderTy &Builder,
+                                         CodeGenModule &CGM) {
+  llvm::Value *GEPIndices[] = {Builder.getInt32(0), Builder.getInt32(0)};
+  return loadRISCVFeatureBitsCommon(GEPIndices, Builder, CGM);
+}
+
+static Value *loadRISCVFeatureBits(unsigned Index, CGBuilderTy &Builder,
+                                   CodeGenModule &CGM) {
+  llvm::Type *Int32Ty = Builder.getInt32Ty();
+  Value *IndexVal = llvm::ConstantInt::get(Int32Ty, Index);
+  llvm::Value *GEPIndices[] = {Builder.getInt32(0), Builder.getInt32(1),
+                               IndexVal};
+  return loadRISCVFeatureBitsCommon(GEPIndices, Builder, CGM);
+}
+
+llvm::Value *CodeGenFunction::EmitRISCVFeatureBitsLengthCond() {
+  return Builder.CreateICmpULE(
+      loadRISCVFeatureBitsLength(Builder, CGM),
+      Builder.getInt64(llvm::RISCVISAInfo::FeatureBitSize));
 }
 
 Value *CodeGenFunction::EmitRISCVCpuSupports(ArrayRef<StringRef> FeaturesStrs) {
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index eda96f3e352ce3..9e558195aa6daa 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2945,6 +2945,10 @@ void CodeGenFunction::EmitRISCVMultiVersionResolver(
                getPriorityFromAttrString(RHS.Conditions.Features[0]);
       });
 
+  llvm::BasicBlock *LengthBlock = CurBlock;
+  llvm::BasicBlock *VersionBlock = createBasicBlock("version_begin", Resolver);
+  CurBlock = VersionBlock;
+
   // Check the each candidate function.
   for (unsigned Index = 0; Index < CurrOptions.size(); Index++) {
 
@@ -2970,22 +2974,28 @@ void CodeGenFunction::EmitRISCVMultiVersionResolver(
     // (__riscv_feature_bits.features[i] & REQUIRED_BITMASK) ==
     // REQUIRED_BITMASK
     //
+    // First, check __riscv_feature_bits.length <=
+    // llvm::RISCVISAInfo::FeatureBitSize. This ensures that the
+    // __riscv_feature_bits object at runtime has the same length as on the
+    // compiler side.
+    //
+    // Second, 
     // When condition is met, return this version of the function.
     // Otherwise, try the next version.
     //
-    // if (FeaturesConditionVersion1)
+    //
+    // if (__riscv_feature_bits.features.length <=
+    // llvm::RISCVISAInfo::FeatureBitSize) {
+    //   if (FeaturesConditionVersion1)
     //     return Version1;
-    // else if (FeaturesConditionVersion2)
+    //   else if (FeaturesConditionVersion2)
     //     return Version2;
-    // else if (FeaturesConditionVersion3)
+    //   else if (FeaturesConditionVersion3)
     //     return Version3;
-    // ...
-    // else
-    //     return DefaultVersion;
+    //   ...
+    // }
+    // return DefaultVersion;
 
-    // TODO: Add a condition to check the length before accessing elements.
-    // Without checking the length first, we may access an incorrect memory
-    // address when using different versions.
     llvm::SmallVector<StringRef, 8> CurrTargetAttrFeats;
 
     for (auto &Feat : TargetAttrFeats) {
@@ -3009,6 +3019,10 @@ void CodeGenFunction::EmitRISCVMultiVersionResolver(
     CurBlock = ElseBlock;
   }
 
+  Builder.SetInsertPoint(LengthBlock);
+  llvm::Value *FeatsLengthCond = EmitRISCVFeatureBitsLengthCond();
+  Builder.CreateCondBr(FeatsLengthCond, VersionBlock, CurBlock);
+
   // Finally, emit the default one.
   if (HasDefault) {
     Builder.SetInsertPoint(CurBlock);
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 3e2abbd9bc1094..5e1cb1a6e421cb 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4715,6 +4715,7 @@ class CodeGenFunction : public CodeGenTypeCache {
 
   llvm::Value *EmitRISCVCpuSupports(const CallExpr *E);
   llvm::Value *EmitRISCVCpuSupports(ArrayRef<StringRef> FeaturesStrs);
+  llvm::Value *EmitRISCVFeatureBitsLengthCond();
   llvm::Value *EmitRISCVCpuInit();
 
   void AddAMDGPUFenceAddressSpaceMMRA(llvm::Instruction *Inst,
diff --git a/clang/test/CodeGen/attr-target-clones-riscv.c b/clang/test/CodeGen/attr-target-clones-riscv.c
index 4a5dea91e22769..47900effb673ea 100644
--- a/clang/test/CodeGen/attr-target-clones-riscv.c
+++ b/clang/test/CodeGen/attr-target-clones-riscv.c
@@ -59,10 +59,14 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo1.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 4096
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 4096
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 4096
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 4096
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo1._m
 // CHECK:       resolver_else:
@@ -90,17 +94,21 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo2.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 268435456
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 268435456
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE2:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 268435456
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 268435456
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo2._zbb
 // CHECK:       resolver_else:
-// CHECK-NEXT:    [[TMP3:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP4:%.*]] = and i64 [[TMP3]], 4096
-// CHECK-NEXT:    [[TMP5:%.*]] = icmp eq i64 [[TMP4]], 4096
-// CHECK-NEXT:    br i1 [[TMP5]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2:%.*]]
+// CHECK-NEXT:    [[TMP5:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP6:%.*]] = and i64 [[TMP5]], 4096
+// CHECK-NEXT:    [[TMP7:%.*]] = icmp eq i64 [[TMP6]], 4096
+// CHECK-NEXT:    br i1 [[TMP7]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2]]
 // CHECK:       resolver_return1:
 // CHECK-NEXT:    ret ptr @foo2._m
 // CHECK:       resolver_else2:
@@ -122,10 +130,14 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo3.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 268435460
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 268435460
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 268435460
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 268435460
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo3._c_zbb
 // CHECK:       resolver_else:
@@ -147,10 +159,14 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo4.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 270532608
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 270532608
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 270532608
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 270532608
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo4._v_zbb
 // CHECK:       resolver_else:
@@ -166,6 +182,10 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo5.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[TRY_VERSION]]
+// CHECK:       try_version:
 // CHECK-NEXT:    ret ptr @foo5.default
 //
 //
@@ -184,10 +204,14 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo6.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 576460752303423488
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 576460752303423488
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 576460752303423488
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 576460752303423488
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo6._zvkt
 // CHECK:       resolver_else:
@@ -221,24 +245,28 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo7.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 268435456
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 268435456
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE4:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 268435456
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 268435456
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo7._zbb
 // CHECK:       resolver_else:
-// CHECK-NEXT:    [[TMP3:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP4:%.*]] = and i64 [[TMP3]], 134217728
-// CHECK-NEXT:    [[TMP5:%.*]] = icmp eq i64 [[TMP4]], 134217728
-// CHECK-NEXT:    br i1 [[TMP5]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2:%.*]]
+// CHECK-NEXT:    [[TMP5:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP6:%.*]] = and i64 [[TMP5]], 134217728
+// CHECK-NEXT:    [[TMP7:%.*]] = icmp eq i64 [[TMP6]], 134217728
+// CHECK-NEXT:    br i1 [[TMP7]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2:%.*]]
 // CHECK:       resolver_return1:
 // CHECK-NEXT:    ret ptr @foo7._zba
 // CHECK:       resolver_else2:
-// CHECK-NEXT:    [[TMP6:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP7:%.*]] = and i64 [[TMP6]], 402653184
-// CHECK-NEXT:    [[TMP8:%.*]] = icmp eq i64 [[TMP7]], 402653184
-// CHECK-NEXT:    br i1 [[TMP8]], label [[RESOLVER_RETURN3:%.*]], label [[RESOLVER_ELSE4:%.*]]
+// CHECK-NEXT:    [[TMP8:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP9:%.*]] = and i64 [[TMP8]], 402653184
+// CHECK-NEXT:    [[TMP10:%.*]] = icmp eq i64 [[TMP9]], 402653184
+// CHECK-NEXT:    br i1 [[TMP10]], label [[RESOLVER_RETURN3:%.*]], label [[RESOLVER_ELSE4]]
 // CHECK:       resolver_return3:
 // CHECK-NEXT:    ret ptr @foo7._zba_zbb
 // CHECK:       resolver_else4:
@@ -272,24 +300,28 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo8.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 402653184
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 402653184
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE4:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 402653184
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 402653184
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo8._zba_zbb
 // CHECK:       resolver_else:
-// CHECK-NEXT:    [[TMP3:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP4:%.*]] = and i64 [[TMP3]], 268435456
-// CHECK-NEXT:    [[TMP5:%.*]] = icmp eq i64 [[TMP4]], 268435456
-// CHECK-NEXT:    br i1 [[TMP5]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2:%.*]]
+// CHECK-NEXT:    [[TMP5:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP6:%.*]] = and i64 [[TMP5]], 268435456
+// CHECK-NEXT:    [[TMP7:%.*]] = icmp eq i64 [[TMP6]], 268435456
+// CHECK-NEXT:    br i1 [[TMP7]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2:%.*]]
 // CHECK:       resolver_return1:
 // CHECK-NEXT:    ret ptr @foo8._zbb
 // CHECK:       resolver_else2:
-// CHECK-NEXT:    [[TMP6:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP7:%.*]] = and i64 [[TMP6]], 134217728
-// CHECK-NEXT:    [[TMP8:%.*]] = icmp eq i64 [[TMP7]], 134217728
-// CHECK-NEXT:    br i1 [[TMP8]], label [[RESOLVER_RETURN3:%.*]], label [[RESOLVER_ELSE4:%.*]]
+// CHECK-NEXT:    [[TMP8:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP9:%.*]] = and i64 [[TMP8]], 134217728
+// CHECK-NEXT:    [[TMP10:%.*]] = icmp eq i64 [[TMP9]], 134217728
+// CHECK-NEXT:    br i1 [[TMP10]], label [[RESOLVER_RETURN3:%.*]], label [[RESOLVER_ELSE4]]
 // CHECK:       resolver_return3:
 // CHECK-NEXT:    ret ptr @foo8._zba
 // CHECK:       resolver_else4:
@@ -323,24 +355,28 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo9.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] ...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Sep 26, 2024

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

Author: Piyou Chen (BeMg)

Changes

This patch add a condition that check the __riscv_feature_bits.length before access __riscv_feature_bits.features.

It updates the resolver function as the following structure.

if (__riscv_feature_bits.features.length &lt;=
    llvm::RISCVISAInfo::FeatureBitSize) {
    if (FeaturesConditionVersion1)
        return Version1;
    else if (FeaturesConditionVersion2)
        return Version2;
    else if (FeaturesConditionVersion3)
        return Version3;
    ...
}
return DefaultVersion;

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

5 Files Affected:

  • (modified) clang/lib/CodeGen/CGBuiltin.cpp (+26-7)
  • (modified) clang/lib/CodeGen/CodeGenFunction.cpp (+23-9)
  • (modified) clang/lib/CodeGen/CodeGenFunction.h (+1)
  • (modified) clang/test/CodeGen/attr-target-clones-riscv.c (+112-72)
  • (modified) clang/test/CodeGenCXX/attr-target-clones-riscv.cpp (+112-72)
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 249aead33ad73d..6af6f166575142 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -14534,8 +14534,9 @@ Value *CodeGenFunction::EmitRISCVCpuSupports(const CallExpr *E) {
   return EmitRISCVCpuSupports(ArrayRef<StringRef>(FeatureStr));
 }
 
-static Value *loadRISCVFeatureBits(unsigned Index, CGBuilderTy &Builder,
-                                   CodeGenModule &CGM) {
+static Value *loadRISCVFeatureBitsCommon(ArrayRef<llvm::Value *> GEPIndices,
+                                         CGBuilderTy &Builder,
+                                         CodeGenModule &CGM) {
   llvm::Type *Int32Ty = Builder.getInt32Ty();
   llvm::Type *Int64Ty = Builder.getInt64Ty();
   llvm::ArrayType *ArrayOfInt64Ty =
@@ -14544,14 +14545,32 @@ static Value *loadRISCVFeatureBits(unsigned Index, CGBuilderTy &Builder,
   llvm::Constant *RISCVFeaturesBits =
       CGM.CreateRuntimeVariable(StructTy, "__riscv_feature_bits");
   cast<llvm::GlobalValue>(RISCVFeaturesBits)->setDSOLocal(true);
-  Value *IndexVal = llvm::ConstantInt::get(Int32Ty, Index);
-  llvm::Value *GEPIndices[] = {Builder.getInt32(0), Builder.getInt32(1),
-                               IndexVal};
   Value *Ptr =
       Builder.CreateInBoundsGEP(StructTy, RISCVFeaturesBits, GEPIndices);
-  Value *FeaturesBit =
+  Value *FeaturesVal =
       Builder.CreateAlignedLoad(Int64Ty, Ptr, CharUnits::fromQuantity(8));
-  return FeaturesBit;
+  return FeaturesVal;
+}
+
+static Value *loadRISCVFeatureBitsLength(CGBuilderTy &Builder,
+                                         CodeGenModule &CGM) {
+  llvm::Value *GEPIndices[] = {Builder.getInt32(0), Builder.getInt32(0)};
+  return loadRISCVFeatureBitsCommon(GEPIndices, Builder, CGM);
+}
+
+static Value *loadRISCVFeatureBits(unsigned Index, CGBuilderTy &Builder,
+                                   CodeGenModule &CGM) {
+  llvm::Type *Int32Ty = Builder.getInt32Ty();
+  Value *IndexVal = llvm::ConstantInt::get(Int32Ty, Index);
+  llvm::Value *GEPIndices[] = {Builder.getInt32(0), Builder.getInt32(1),
+                               IndexVal};
+  return loadRISCVFeatureBitsCommon(GEPIndices, Builder, CGM);
+}
+
+llvm::Value *CodeGenFunction::EmitRISCVFeatureBitsLengthCond() {
+  return Builder.CreateICmpULE(
+      loadRISCVFeatureBitsLength(Builder, CGM),
+      Builder.getInt64(llvm::RISCVISAInfo::FeatureBitSize));
 }
 
 Value *CodeGenFunction::EmitRISCVCpuSupports(ArrayRef<StringRef> FeaturesStrs) {
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index eda96f3e352ce3..9e558195aa6daa 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2945,6 +2945,10 @@ void CodeGenFunction::EmitRISCVMultiVersionResolver(
                getPriorityFromAttrString(RHS.Conditions.Features[0]);
       });
 
+  llvm::BasicBlock *LengthBlock = CurBlock;
+  llvm::BasicBlock *VersionBlock = createBasicBlock("version_begin", Resolver);
+  CurBlock = VersionBlock;
+
   // Check the each candidate function.
   for (unsigned Index = 0; Index < CurrOptions.size(); Index++) {
 
@@ -2970,22 +2974,28 @@ void CodeGenFunction::EmitRISCVMultiVersionResolver(
     // (__riscv_feature_bits.features[i] & REQUIRED_BITMASK) ==
     // REQUIRED_BITMASK
     //
+    // First, check __riscv_feature_bits.length <=
+    // llvm::RISCVISAInfo::FeatureBitSize. This ensures that the
+    // __riscv_feature_bits object at runtime has the same length as on the
+    // compiler side.
+    //
+    // Second, 
     // When condition is met, return this version of the function.
     // Otherwise, try the next version.
     //
-    // if (FeaturesConditionVersion1)
+    //
+    // if (__riscv_feature_bits.features.length <=
+    // llvm::RISCVISAInfo::FeatureBitSize) {
+    //   if (FeaturesConditionVersion1)
     //     return Version1;
-    // else if (FeaturesConditionVersion2)
+    //   else if (FeaturesConditionVersion2)
     //     return Version2;
-    // else if (FeaturesConditionVersion3)
+    //   else if (FeaturesConditionVersion3)
     //     return Version3;
-    // ...
-    // else
-    //     return DefaultVersion;
+    //   ...
+    // }
+    // return DefaultVersion;
 
-    // TODO: Add a condition to check the length before accessing elements.
-    // Without checking the length first, we may access an incorrect memory
-    // address when using different versions.
     llvm::SmallVector<StringRef, 8> CurrTargetAttrFeats;
 
     for (auto &Feat : TargetAttrFeats) {
@@ -3009,6 +3019,10 @@ void CodeGenFunction::EmitRISCVMultiVersionResolver(
     CurBlock = ElseBlock;
   }
 
+  Builder.SetInsertPoint(LengthBlock);
+  llvm::Value *FeatsLengthCond = EmitRISCVFeatureBitsLengthCond();
+  Builder.CreateCondBr(FeatsLengthCond, VersionBlock, CurBlock);
+
   // Finally, emit the default one.
   if (HasDefault) {
     Builder.SetInsertPoint(CurBlock);
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 3e2abbd9bc1094..5e1cb1a6e421cb 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4715,6 +4715,7 @@ class CodeGenFunction : public CodeGenTypeCache {
 
   llvm::Value *EmitRISCVCpuSupports(const CallExpr *E);
   llvm::Value *EmitRISCVCpuSupports(ArrayRef<StringRef> FeaturesStrs);
+  llvm::Value *EmitRISCVFeatureBitsLengthCond();
   llvm::Value *EmitRISCVCpuInit();
 
   void AddAMDGPUFenceAddressSpaceMMRA(llvm::Instruction *Inst,
diff --git a/clang/test/CodeGen/attr-target-clones-riscv.c b/clang/test/CodeGen/attr-target-clones-riscv.c
index 4a5dea91e22769..47900effb673ea 100644
--- a/clang/test/CodeGen/attr-target-clones-riscv.c
+++ b/clang/test/CodeGen/attr-target-clones-riscv.c
@@ -59,10 +59,14 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo1.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 4096
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 4096
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 4096
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 4096
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo1._m
 // CHECK:       resolver_else:
@@ -90,17 +94,21 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo2.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 268435456
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 268435456
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE2:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 268435456
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 268435456
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo2._zbb
 // CHECK:       resolver_else:
-// CHECK-NEXT:    [[TMP3:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP4:%.*]] = and i64 [[TMP3]], 4096
-// CHECK-NEXT:    [[TMP5:%.*]] = icmp eq i64 [[TMP4]], 4096
-// CHECK-NEXT:    br i1 [[TMP5]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2:%.*]]
+// CHECK-NEXT:    [[TMP5:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP6:%.*]] = and i64 [[TMP5]], 4096
+// CHECK-NEXT:    [[TMP7:%.*]] = icmp eq i64 [[TMP6]], 4096
+// CHECK-NEXT:    br i1 [[TMP7]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2]]
 // CHECK:       resolver_return1:
 // CHECK-NEXT:    ret ptr @foo2._m
 // CHECK:       resolver_else2:
@@ -122,10 +130,14 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo3.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 268435460
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 268435460
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 268435460
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 268435460
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo3._c_zbb
 // CHECK:       resolver_else:
@@ -147,10 +159,14 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo4.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 270532608
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 270532608
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 270532608
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 270532608
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo4._v_zbb
 // CHECK:       resolver_else:
@@ -166,6 +182,10 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo5.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[TRY_VERSION]]
+// CHECK:       try_version:
 // CHECK-NEXT:    ret ptr @foo5.default
 //
 //
@@ -184,10 +204,14 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo6.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 576460752303423488
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 576460752303423488
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 576460752303423488
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 576460752303423488
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo6._zvkt
 // CHECK:       resolver_else:
@@ -221,24 +245,28 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo7.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 268435456
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 268435456
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE4:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 268435456
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 268435456
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo7._zbb
 // CHECK:       resolver_else:
-// CHECK-NEXT:    [[TMP3:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP4:%.*]] = and i64 [[TMP3]], 134217728
-// CHECK-NEXT:    [[TMP5:%.*]] = icmp eq i64 [[TMP4]], 134217728
-// CHECK-NEXT:    br i1 [[TMP5]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2:%.*]]
+// CHECK-NEXT:    [[TMP5:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP6:%.*]] = and i64 [[TMP5]], 134217728
+// CHECK-NEXT:    [[TMP7:%.*]] = icmp eq i64 [[TMP6]], 134217728
+// CHECK-NEXT:    br i1 [[TMP7]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2:%.*]]
 // CHECK:       resolver_return1:
 // CHECK-NEXT:    ret ptr @foo7._zba
 // CHECK:       resolver_else2:
-// CHECK-NEXT:    [[TMP6:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP7:%.*]] = and i64 [[TMP6]], 402653184
-// CHECK-NEXT:    [[TMP8:%.*]] = icmp eq i64 [[TMP7]], 402653184
-// CHECK-NEXT:    br i1 [[TMP8]], label [[RESOLVER_RETURN3:%.*]], label [[RESOLVER_ELSE4:%.*]]
+// CHECK-NEXT:    [[TMP8:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP9:%.*]] = and i64 [[TMP8]], 402653184
+// CHECK-NEXT:    [[TMP10:%.*]] = icmp eq i64 [[TMP9]], 402653184
+// CHECK-NEXT:    br i1 [[TMP10]], label [[RESOLVER_RETURN3:%.*]], label [[RESOLVER_ELSE4]]
 // CHECK:       resolver_return3:
 // CHECK-NEXT:    ret ptr @foo7._zba_zbb
 // CHECK:       resolver_else4:
@@ -272,24 +300,28 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo8.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 402653184
-// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 402653184
-// CHECK-NEXT:    br i1 [[TMP2]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__riscv_feature_bits, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = icmp ule i64 [[TMP0]], 2
+// CHECK-NEXT:    br i1 [[TMP1]], label [[TRY_VERSION:%.*]], label [[RESOLVER_ELSE4:%.*]]
+// CHECK:       try_version:
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP2]], 402653184
+// CHECK-NEXT:    [[TMP4:%.*]] = icmp eq i64 [[TMP3]], 402653184
+// CHECK-NEXT:    br i1 [[TMP4]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
 // CHECK:       resolver_return:
 // CHECK-NEXT:    ret ptr @foo8._zba_zbb
 // CHECK:       resolver_else:
-// CHECK-NEXT:    [[TMP3:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP4:%.*]] = and i64 [[TMP3]], 268435456
-// CHECK-NEXT:    [[TMP5:%.*]] = icmp eq i64 [[TMP4]], 268435456
-// CHECK-NEXT:    br i1 [[TMP5]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2:%.*]]
+// CHECK-NEXT:    [[TMP5:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP6:%.*]] = and i64 [[TMP5]], 268435456
+// CHECK-NEXT:    [[TMP7:%.*]] = icmp eq i64 [[TMP6]], 268435456
+// CHECK-NEXT:    br i1 [[TMP7]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2:%.*]]
 // CHECK:       resolver_return1:
 // CHECK-NEXT:    ret ptr @foo8._zbb
 // CHECK:       resolver_else2:
-// CHECK-NEXT:    [[TMP6:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
-// CHECK-NEXT:    [[TMP7:%.*]] = and i64 [[TMP6]], 134217728
-// CHECK-NEXT:    [[TMP8:%.*]] = icmp eq i64 [[TMP7]], 134217728
-// CHECK-NEXT:    br i1 [[TMP8]], label [[RESOLVER_RETURN3:%.*]], label [[RESOLVER_ELSE4:%.*]]
+// CHECK-NEXT:    [[TMP8:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [2 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
+// CHECK-NEXT:    [[TMP9:%.*]] = and i64 [[TMP8]], 134217728
+// CHECK-NEXT:    [[TMP10:%.*]] = icmp eq i64 [[TMP9]], 134217728
+// CHECK-NEXT:    br i1 [[TMP10]], label [[RESOLVER_RETURN3:%.*]], label [[RESOLVER_ELSE4]]
 // CHECK:       resolver_return3:
 // CHECK-NEXT:    ret ptr @foo8._zba
 // CHECK:       resolver_else4:
@@ -323,24 +355,28 @@ int bar() { return foo1() + foo2() + foo3() + foo4() + foo5() + foo6() + foo7()
 // CHECK-LABEL: define weak_odr ptr @foo9.resolver() comdat {
 // CHECK-NEXT:  resolver_entry:
 // CHECK-NEXT:    call void @__init_riscv_feature_bits(ptr null)
-// CHECK-NEXT:    [[TMP0:%.*]] ...
[truncated]

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 26, 2024 •
edited
Loading

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

jrtc27
jrtc27 reviewed Sep 26, 2024
View reviewed changes
clang/lib/CodeGen/CodeGenFunction.cpp
//
// if (FeaturesConditionVersion1)
//
// if (__riscv_feature_bits.features.length <=
Copy link
Collaborator

@jrtc27 jrtc27 Sep 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

>=, surely? But also this should be per set of features being tested, and specific to the min length needed for that, not apply to all versions. Otherwise if the compiler learns about a new word but is run on an older OS it will stop using optimised versions.

@preames
Copy link
Collaborator

preames commented Sep 26, 2024

Why? There has only ever been one version of the features array, and that has not yet been published. Why do we need a version check here at all?

@BeMg
Copy link
Contributor Author

BeMg commented Sep 27, 2024

There is misunderstand here. Close it now and reopen when it ready.

@BeMg BeMg closed this Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrtc27 jrtc27 jrtc27 left review comments

@preames preames Awaiting requested review from preames

@kito-cheng kito-cheng Awaiting requested review from kito-cheng

@topperc topperc Awaiting requested review from topperc

Assignees
No one assigned
Labels
backend:RISC-V clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@BeMg @llvmbot @preames @jrtc27