Closed Bug 716766 Opened 11 years ago Closed 9 years ago

[Skia] Add runtime detection of NEON support


(Core :: Graphics, defect)

Not set





(Reporter: mattwoodrow, Assigned: gw280)




(Whiteboard: [ARM-opt])


(2 files, 2 obsolete files)

We should modify skia so that we can compile with NEON enabled, but still run on devices that don't support it.

There is an existing (badly rotted) patch at:

Anyone know enough NEON to attempt getting this working again?
It appears that our existing code for doing this on android-ish is .  It would be cool if skia had a hook we could use to implemen NEON detection with our existing arm.h code, but something tells me that might not be a popular option upstream.
Attachment #743454 - Flags: review?(gwright)
Assignee: nobody → milan
We can't quite do mandatory because we run on some devices that don't support NEON, but James has the details.
Assignee: milan → snorp
We need to check with the Skia guys to figure out what's best here. I don't know how the optional neon bits work there.
George, can you contact Skia people and ask them about this?
Flags: needinfo?(gwright)
Upstream should have resolved this issue by now, but I will double check with them. Leaving the needinfo open for me as I don't want to address this until we've updated our Skia revision.
Comment on attachment 743454 [details] [diff] [review]

Review of attachment 743454 [details] [diff] [review]:

I'm going to look at this now as it shouldn't take too long, and we've finally updated our skia rev. r- mainly because we modify now instead of
Attachment #743454 - Flags: review?(gwright) → review-
Assignee: snorp → gwright
The fonthost hunks in the change to are due to regenerating after I created this patch:

The rest should be fairly self explanatory.
Attachment #743454 - Attachment is obsolete: true
Attachment #8387891 - Flags: review?(snorp)
I think -mfpu=neon allows the compiler to use neon instructions whenever it wants, which is bad (since we run on devices without neon). I am not sure, though.
No, it's correct, because those files are only ever used if Skia detects NEON is available at runtime.
Looks like armv6 is busted, otherwise I guess I'm ok with this.
Yep, looks like I just need to add the neon files in:


And it'll all work fine. Just doing a local build test now.
Use BUILD_ARM_NEON to protect the NEON files.
Attachment #8387891 - Attachment is obsolete: true
Attachment #8387891 - Flags: review?(snorp)
Attachment #8388811 - Flags: review?(snorp)
Attachment #8388811 - Flags: review?(snorp) → review+
Attachment #8387890 - Flags: review?(snorp) → review+
Relevant upstream bug: (so the patch won't need to be held locally)
I don't think the crashes are related to this patch. snorp, can you give a second opinion?
Flags: needinfo?(gwright) → needinfo?(snorp)
(In reply to George Wright (:gw280) from comment #21)
> I don't think the crashes are related to this patch. snorp, can you give a
> second opinion?

Yeah, doesn't look likely to me.
Flags: needinfo?(snorp)
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Duplicate of this bug: 792924
You need to log in before you can comment on or make changes to this bug.