Skip to content
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

config: enable auto detection by default #80

Merged
merged 1 commit into from
Aug 28, 2020
Merged

Conversation

carenas
Copy link
Contributor

@carenas carenas commented Aug 13, 2020

instead of needing -DSLJIT_CONFIG_AUTO or some other definition, do auto detection unless a specific definition was passed or it was explicitly disabled by -DSLJIT_CONFIG_AUTO=0

in platforms (ex: armel or armhf) where there might be multiple possible targets to chose (ex: thumb2, v5 or v7) allows reconfiguring the build without having to edit the makefile, for example:

$ make EXTRA_CPPFLAGS=-DSLJIT_CONFIG_ARM_THUMB2

additionally, this allows to generate code for a different (but compatible) ABI (ex: ARM EABI 5 application generating Thumb2 code in an arm64 CPU/OS)

Copy link
Owner

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

The problem with making auto automatic, that people may misconfigure the engine unintentionally.

Copy link
Owner

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

Maybe the steps could be the following:

#if !(ANY_ARCH_OR_AUTO_DEFINED_AND_SET)
#if AUTO_DEFINED_AND_DISABLED
#error "An arch must be selected"
#else
define auto
#endif
#endif

@carenas carenas marked this pull request as draft August 27, 2020 09:09
@zherczeg
Copy link
Owner

The last variant is still incorrect. You can define these with the value of 0, and that is the same thing as being not defined. I have proposed a variant above, that should not have this weakness.

@carenas carenas force-pushed the auto branch 2 times, most recently from 1f6bfa6 to 1abc0ec Compare August 27, 2020 10:13
@carenas carenas marked this pull request as ready for review August 27, 2020 10:13
@carenas carenas requested a review from zherczeg August 27, 2020 10:25
@carenas carenas force-pushed the auto branch 2 times, most recently from 7234680 to d19da41 Compare August 27, 2020 16:43
@carenas carenas changed the title config: enable auto by default config: enable auto config by default Aug 27, 2020
@carenas carenas changed the title config: enable auto config by default config: enable auto detection by default Aug 27, 2020
instead of requiring -DSLJIT_CONFIG_AUTO to enable, do autodetection
unless a specific definition was used or auto detection was explicitly
disabled by using -DSLJIT_CONFIG_AUTO=0
Copy link
Owner

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM

@zherczeg zherczeg merged commit 502744f into zherczeg:master Aug 28, 2020
fengjixuchui added a commit to fengjixuchui/sljit that referenced this pull request Aug 28, 2020
config: enable auto detection by default (zherczeg#80)
@carenas carenas deleted the auto branch August 28, 2020 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants