Skip to content
This repository has been archived by the owner. It is now read-only.

Enhance the detection of SSE2-compatible targets #132

Merged
merged 1 commit into from Jan 18, 2015

Conversation

@akosthekiss
Copy link
Contributor

akosthekiss commented Jan 14, 2015

Assuming that any target that doesn't have "arm" in its name is i686 or x86_64
and thus SSE2-compatible is incorrect (counter example: aarch64). It's better to
look for "86", which appears in the name of all 8086 descendants.

@mrobinson
Copy link
Member

mrobinson commented Jan 14, 2015

I wonder if it would make sense to simply enumerate the SSE targets?

@akosthekiss
Copy link
Contributor Author

akosthekiss commented Jan 14, 2015

You mean like

ifeq (i686,$(findstring i686,$(TARGET)))
        supports_sse = true
endif
ifeq (x86_64,$(findstring x86_64,$(TARGET)))
        supports_sse = true
endif
ifeq (true,$(supports_sse))
        """things go here"""
endif

?
(I just didn't dare to introduce too big a change. But can update the patch if you ask/agree.)

@mrobinson
Copy link
Member

mrobinson commented Jan 14, 2015

It might be more fool-proof.

Assuming that any target that doesn't have "arm" in its name is an i686 or x86_64
and thus SSE2-compatible is incorrect (counter examples: aarch64 or mips). Make
the check as fool-proof as possible.
@akosthekiss akosthekiss force-pushed the akosthekiss:pr-sse2 branch from d92797d to 0614ec7 Jan 14, 2015
@akosthekiss
Copy link
Contributor Author

akosthekiss commented Jan 17, 2015

Does the updated patch look better?

@mrobinson
Copy link
Member

mrobinson commented Jan 18, 2015

Yeah, let's give this a try. Thanks!

mrobinson added a commit that referenced this pull request Jan 18, 2015
Enhance the detection of SSE2-compatible targets
@mrobinson mrobinson merged commit 3080e1b into servo:master Jan 18, 2015
@akosthekiss akosthekiss deleted the akosthekiss:pr-sse2 branch Jan 19, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.