[Proposal] Improve public API

[toor1245]

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.

// 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?

#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:

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?

[gchatelet]

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

Are you talking about porting the library to different languages? Which ones?

however we have one point with shared library

Not only there's a potential issue with bitfields but we are also not providing any ABI stability guarantees. That's why we strongly discourage the use of shared libraries.

When this project was created, the vision was that it could be run on embedded devices with small memory footprint - hence C99 and bitfields.

I'm not fond of the #ifdef/#endif solution which duplicates the code and is bound to diverge over time. Also it makes the header file harder to understand.

---

Now if we want to port the API to higher level languages (C#, Java, Python, Go) I think we can trade performance and use the reflection API instead. This should already work out of the box for features in the following (admittedly ugly) way:

• walk GetX86FeaturesEnumName() by using an incrementing number from 0 up until you get "unknown_feature".
• then you can query all features from 0 to the number you found with GetX86FeaturesEnumValue() which returns a non-0 or 0 value depending on if it's set or not.

Solution 1, a real API

It is not nice but I believe we can create a nicer API on top of that that abstracts cpu away completely. e.g.

int GetLastFeatureEnum();
int GetFeaturesEnumValue(int);
const char* GetFeaturesEnumString(int);

Solution 2, parsing JSON

Another option that requires much less work is to have the host language run list_cpu_features --json and parse its output (or provide a library function to prevent the overhead of creating a process, something like const char* GetJsonCpuInfo()). If we choose this option then we need to properly define the content of the JSON output and make it stable over time.

What do you think?