search

lemire / simdcomp

A simple C library for compressing lists of integers using binary packing
BSD 3-Clause "New" or "Revised" License
488 stars 53 forks source link

C89 compatibility #4

Closed weltling closed 9 years ago

weltling commented 9 years ago

@lemire i think it's finished now also with the strict gcc C89 compat. But actually I'm kind of disappointed with that as the source looks much uglier now, just as you said :) ... Here's the diff https://github.com/lemire/simdcomp/compare/c89_compat . The unit and example progs seem to pass however, also no any significant signs in the performance.

Not sure it should be merged, nevertheless one could keep that branch for people who possibly need C89 compatible code for whatever reasons (mostly compiler bound probably). Or, one could merge it but omit the gcc commit. That were a sort of broken C89 compatibility where the C source would partly follow it, but still would be able to use stdint.h, the always_inline attribute and maybe more. To note is also that gcc and vc are not the only compilers around.

I'm thinking about the real world implementation and particularly about implementing some PHP extension. There one is not only bound to the just compressing data, but can also implementing data stream compression, data serialization, beyond. However would probably reuse the C89 branch for that, as mentioned earlier, PHP sticks to C89 (but more like in that broken C89 variant ATM).

Thanks.

lemire commented 9 years ago

@weltling I am curious as to what you find ugly. I browsed all of the diffs, and I do not see much ugliness.

The ugly part I was thinking about is that I do not think C89 had the uint32_t type. This became a requirement with C99. Of course, I think MSVC had stdint.h as for 2010... but strictly speaking, it was an extension to the standard at this point.

Of course, we have no choice but to go beyond the standard... as C89 does not have Intel intrinsics for SIMD instructions. ;-)

I suggest that we merge this unless you have other concerns than a vague sense of ugliness.

weltling commented 9 years ago

I've found ugly switching to macros instead of inlined functions, and also not being able using stdint.h ... probably that's too subjective ... some concerns I had was also the rare platforms like SPARC64 or Powerpc64, though i cannot touch any of those ATM, and they might have completely different issues anyway.

Functionally the C89 patch seems to be doing no harm. I've seen gcc having tests for C89 and intrinsics. From that point, the end binary won't diff much. MSVC and gcc are both odd animals :) Resuming, especially as the code quality isn't looking bad to you, I'm going to check this all again and merge.

Cheers :)

lemire commented 9 years ago

Supporting non-x86 processors would bring critical problems... since the code uses Intel intrinsics.

weltling commented 9 years ago

yeah, fair enough ... probably more a thought issue as a real one. So keeping KISS )

weltling commented 9 years ago

Merged.