-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
[cmake] addons: windows: only add sse2 flags for x86 #23214
Conversation
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
Fine but commit/PR title seems confusing: You cannot "remove SSE2 support" from x64 because SSE2 instructions are supported on all x64 processors. Then maybe something like: "Remove /SSE2 build flag from x64 as SSE2 support is already enabled by default" or "remove unnecessary /SSE2 build flag from x64" |
I guess https://github.com/xbmc/xbmc/blob/master/cmake/modules/FindSSE.cmake#L130-L137 is redundant |
Signed-off-by: Lukas Rusak <lorusak@gmail.com>
So I think there is some misunderstanding here as this only effects the add-on build IIRC. These scripts only have some variables available.
x64
x86
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes are correct and although variable value is x86
in commit name it would be more correct to talk about win32
because x86 is both 32 and 64 bits
well it seems that windows refers to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With more less warnings is always good, thanks about the fix!
I noticed that the build log for win64 has a lot of lines like this:
This should clear that up
ref: https://stackoverflow.com/questions/1067630/sse2-option-in-visual-c-x64
What is the effect on users?
less spam in the build log?