google / google/cpu_features

[Proposal] Improve public API

Open
#217 1 comment 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
C++
Stars
2.6k
Forks
309
Avg merge
6d 58m
Merged PRs (30d)
2

Description

It would be great to improve the public API for porting the library.

My proposal is to add macro `CPU_FEATURES_EXPORT` to `include/cpu_features_macros.h` and add to public functions this export macro.
```C
// include/cpu_features_macros.h
#if defined(CPU_FEATURES_COMPILER_MSC)
#if !defined(CPU_FEATURES_SHARED_LIB)
#define CPU_FEATURES_EXPORT
#elif defined(CPU_FEATURES_SHARED_LIB_EXPORT)
#define CPU_FEATURES_EXPORT __declspec(dllexport)
#else
#define CPU_FEATURES_EXPORT __declspec(dllimport)
#endif
#elif defined(CPU_FEATURES_COMPILER_GCC)
#if defined(CPU_FEATURES_SHARED_LIB) && defined(CPU_FEATURES_SHARED_LIB_EXPORT)
#define CPU_FEATURES_EXPORT __attribute__((visibility("default")))
#else
#define CPU_FEATURES_EXPORT
#endif
#else
#define CPU_FEATURES_EXPORT
#endif

// cpuinfo_x86.h
CPU_FEATURES_EXPORT X86Info GetX86Info(void);
```

however we have one point with shared library
https://github.com/google/cpu_features/blob/149916384b7d94282bd647e19afa1c9df79f2dbd/CMakeLists.txt#L20-L31

Can we provide two structures with bit-fields and without for shared lib?

```C
#if define(CPU_FEATURES_SHARED_LIB) && defined(CPU_FEATURES_SHARED_LIB_EXPORT)
typedef struct {
int msa; // MIPS SIMD Architecture
// https://www.mips.com/products/architectures/ase/simd/
int eva; // Enhanced Virtual Addressing
// https://www.mips.com/products/architectures/mips64/
int r6; // True if is release 6 of the processor.

// Make sure to update MipsFeaturesEnum below if you add a field here.
} MipsFeatures;
#else
typedef struct {
int msa : 1; // MIPS SIMD Architecture
// https://www.mips.com/products/architectures/ase/simd/
int eva : 1; // Enhanced Virtual Addressing
// https://www.mips.com/products/architectures/mips64/
int r6 : 1; // True if is release 6 of the processor.

// Make sure to update MipsFeaturesEnum below if you add a field here.
} MipsFeatures;
#endif
```
or we can do it via bit operations, but this approach is too complicated and requires a lot of work:

```C
typedef struct {
int record;
} MipsFeatures;

#define CPU_FEATURES_MIPS_MSA_FLAG_PRESENT 0x4

int GetFlagMsa(const MipsFeatures* mipsFeatures) {
return IsBitSet(mipsFeatures->record, 3);
}

void SetFlagMsa(const MipsFeatures* mipsFeatures) {
mipsFeatures->record |= CPU_FEATURES_MIPS_MSA_FLAG_PRESENT;
}

void ClearFlagMsa(const MipsFeatures* mipsFeatures) {
mipsFeatures->record &= ~CPU_FEATURES_MIPS_MSA_FLAG_PRESENT;
}
```
@gchatelet, @Mizux, what do you think?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.