Skip to content

[RISCV][compiler-rt] Update __init_riscv_feature_bits prototype #101472

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

Merged
merged 1 commit into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions clang/lib/CodeGen/CGBuiltin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14300,13 +14300,13 @@ Value *CodeGenFunction::EmitAArch64CpuInit() {
}

Value *CodeGenFunction::EmitRISCVCpuInit() {
llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy, false);
llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy, {VoidPtrTy}, false);
llvm::FunctionCallee Func =
CGM.CreateRuntimeFunction(FTy, "__init_riscv_feature_bits");
auto *CalleeGV = cast<llvm::GlobalValue>(Func.getCallee());
CalleeGV->setDSOLocal(true);
CalleeGV->setDLLStorageClass(llvm::GlobalValue::DefaultStorageClass);
return Builder.CreateCall(Func);
return Builder.CreateCall(Func, {llvm::ConstantPointerNull::get(VoidPtrTy)});
}

Value *CodeGenFunction::EmitX86CpuInit() {
Expand Down
4 changes: 2 additions & 2 deletions clang/test/CodeGen/builtin-cpu-supports.c
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ int test_ppc(int a) {
// CHECK-RV32-NEXT: [[RETVAL:%.*]] = alloca i32, align 4
// CHECK-RV32-NEXT: [[A_ADDR:%.*]] = alloca i32, align 4
// CHECK-RV32-NEXT: store i32 [[A]], ptr [[A_ADDR]], align 4
// CHECK-RV32-NEXT: call void @__init_riscv_feature_bits()
// CHECK-RV32-NEXT: call void @__init_riscv_feature_bits(ptr null)
// CHECK-RV32-NEXT: [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [1 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
// CHECK-RV32-NEXT: [[TMP1:%.*]] = and i64 [[TMP0]], 1
// CHECK-RV32-NEXT: [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 1
Expand Down Expand Up @@ -291,7 +291,7 @@ int test_ppc(int a) {
// CHECK-RV64-NEXT: [[RETVAL:%.*]] = alloca i32, align 4
// CHECK-RV64-NEXT: [[A_ADDR:%.*]] = alloca i32, align 4
// CHECK-RV64-NEXT: store i32 [[A]], ptr [[A_ADDR]], align 4
// CHECK-RV64-NEXT: call void @__init_riscv_feature_bits()
// CHECK-RV64-NEXT: call void @__init_riscv_feature_bits(ptr null)
// CHECK-RV64-NEXT: [[TMP0:%.*]] = load i64, ptr getelementptr inbounds ({ i32, [1 x i64] }, ptr @__riscv_feature_bits, i32 0, i32 1, i32 0), align 8
// CHECK-RV64-NEXT: [[TMP1:%.*]] = and i64 [[TMP0]], 1
// CHECK-RV64-NEXT: [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 1
Expand Down
8 changes: 6 additions & 2 deletions compiler-rt/lib/builtins/cpu_model/riscv.c
Original file line number Diff line number Diff line change
Expand Up @@ -280,14 +280,18 @@ static void initRISCVFeature(struct riscv_hwprobe Hwprobes[]) {

static int FeaturesBitCached = 0;

void __init_riscv_feature_bits() CONSTRUCTOR_ATTRIBUTE;
void __init_riscv_feature_bits(void *) CONSTRUCTOR_ATTRIBUTE;

// A constructor function that sets __riscv_feature_bits, and
// __riscv_vendor_feature_bits to the right values. This needs to run
// only once. This constructor is given the highest priority and it should
// run before constructors without the priority set. However, it still runs
// after ifunc initializers and needs to be called explicitly there.
void CONSTRUCTOR_ATTRIBUTE __init_riscv_feature_bits() {

// PlatformArgs allows the platform to provide pre-computed data and access it
// without extra effort. For example, Linux could pass the vDSO object to avoid
// an extra system call.
void CONSTRUCTOR_ATTRIBUTE __init_riscv_feature_bits(void *PlatformArgs) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If it's a constructor, how can it receive a value? And if it's a constructor, why is Clang also calling it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think clang needs to call it because ifunc resolvers may run before constructors?

But I agree having an argument doesn't make sense.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think my point was more "having it be a constructor and called doesn't seem to make a lot of sense", because if the expectation is that every user of the global variable calls the function first then the constructor is pointless, unless it's an "ifunc resolvers must call the function, other users need not" situation? Also, if everyone's calling the function, it could just return the pointer to the data rather than requiring the caller to then get a pointer to the global.


if (FeaturesBitCached)
return;
Expand Down
Loading