SDL icon indicating copy to clipboard operation
SDL copied to clipboard

Adding SDL_Assume macro to give hint to the optimiser

Open devnexen opened this issue 3 years ago • 10 comments

with a given condition generating less instructions.

devnexen avatar Oct 01 '22 07:10 devnexen

Gcc case: __builtin_unreachable requires gcc >= 4.5, and since you add the new macro to a public header then please stricten the condition like: #elif defined(__clang__) || (defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 4))) I don't know since when clang supports __builtin_unreachable - the only version I have is clang-3.4.2 and it just does...

Msvc case: As I remember, __assume() became useful only since VS 2005: https://stackoverflow.com/questions/3096939/ (and no, I can not remember since which version it has been supported.) So, I'd avdise strictening its condition, like: #if defined(_MSC_VER) && (_MSC_VER >= 1400)

sezero avatar Oct 01 '22 12:10 sezero

oh I did not know you supported sdk so far back...

devnexen avatar Oct 01 '22 12:10 devnexen

oh I did not know you supported sdk so far back...

Well, my every-day machine runs on an old CentOS-6.10 and it comes with gcc 4.4.7. I'm one of those said-to-be-extinct dinosaurs :)

sezero avatar Oct 01 '22 13:10 sezero

A silly question from me: can we not just add the assume keywords to SDL_assert itself? Clang's static analyzer already uses SDL_assert as an assumption that the asserted case must be true (that the analyzer_noreturn part in SDL_assert.h).

icculus avatar Nov 23 '22 18:11 icculus

(Err...and add some preprocessor magic to keep the assumptions when assertions are disabled, to be clear.)

icculus avatar Nov 23 '22 18:11 icculus

A silly question from me: can we not just add the assume keywords to SDL_assert itself? Clang's static analyzer already uses SDL_assert as an assumption that the asserted case must be true (that the analyzer_noreturn part in SDL_assert.h).

This seems like a good idea.

slouken avatar Mar 10 '24 16:03 slouken

Ah ignore my last commit I did not see @icculus wanted to take over.

devnexen avatar Mar 10 '24 17:03 devnexen

You can keep going, I haven't looked at this in a year, and just want to make sure I pay attention to it soon. :)

icculus avatar Mar 10 '24 18:03 icculus

The thing is , looking at it now, I m not sure we can just use SDL_assume like SDL_assert ... on clang.

devnexen avatar Mar 10 '24 18:03 devnexen

Do we have any benchmarks showing that this provides any benefit? I'd hate to add something to the API that people will start using to improve their code if it doesn't actually help.

slouken avatar Aug 06 '24 15:08 slouken