Closed
Bug 1260351
Opened 8 years ago
Closed 8 years ago
Image: Enable ConvolveVertically/Horizontally in LS3 MMI
Categories
(Core :: Graphics: ImageLib, defect)
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: hev, Assigned: hev)
Details
Attachments
(2 files, 4 obsolete files)
2.58 KB,
patch
|
tnikkel
:
review+
|
Details | Diff | Splinter Review |
3.15 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
Use skia::ConvolveHorizontally and skia::ConvolveVertically with SIMD on Loongson.
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8735707 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8735710 -
Flags: review?(tnikkel)
Updated•8 years ago
|
Attachment #8735710 -
Flags: review?(tnikkel) → review+
Comment 3•8 years ago
|
||
Nice.
Comment 4•8 years ago
|
||
Comment on attachment 8735707 [details] [diff] [review] 0001-Bug-1260351-MozGlue-Build-Add-mips.h.-r-glandium.patch Review of attachment 8735707 [details] [diff] [review]: ----------------------------------------------------------------- ::: mozglue/build/mips.h @@ +6,5 @@ > + > +#ifndef mozilla_mips_h_ > +#define mozilla_mips_h_ > + > +// for definition of MFBT_DATA You're not using MFBT_DATA :) @@ +12,5 @@ > + > +namespace mozilla { > + > +#ifdef _MIPS_ARCH_LOONGSON3A > + inline bool supports_mmi() { return true; } Can this be detected at runtime?
Attachment #8735707 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 5•8 years ago
|
||
Attachment #8735707 -
Attachment is obsolete: true
Attachment #8736272 -
Flags: review?(mh+mozilla)
Comment 6•8 years ago
|
||
Comment on attachment 8736272 [details] [diff] [review] 0001-Bug-1260351-MozGlue-Build-Add-mips-support.-r-glandi.patch Review of attachment 8736272 [details] [diff] [review]: ----------------------------------------------------------------- ::: mozglue/build/mips.cpp @@ +25,5 @@ > + char buf[1024]; > + memset(buf, 0, sizeof(buf)); > + fread(buf, sizeof(char), sizeof(buf) - 1, fin); > + fclose(fin); > + if (strstr(buf, "Loongson")) Do all loongson have the the extension you want to use? The previous patch seemed to imply not.
Attachment #8736272 -
Flags: review?(mh+mozilla)
Updated•8 years ago
|
Flags: needinfo?(r)
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #6) > Comment on attachment 8736272 [details] [diff] [review] > 0001-Bug-1260351-MozGlue-Build-Add-mips-support.-r-glandi.patch > > Review of attachment 8736272 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mozglue/build/mips.cpp > @@ +25,5 @@ > > + char buf[1024]; > > + memset(buf, 0, sizeof(buf)); > > + fread(buf, sizeof(char), sizeof(buf) - 1, fin); > > + fclose(fin); > > + if (strstr(buf, "Loongson")) > > Do all loongson have the the extension you want to use? The previous patch > seemed to imply not. Oh, No. Only works on Loongson-3 series CPUs. Thanks. ;)
Flags: needinfo?(r)
Assignee | ||
Comment 8•8 years ago
|
||
Attachment #8736272 -
Attachment is obsolete: true
Attachment #8738052 -
Flags: review?(mh+mozilla)
Comment 9•8 years ago
|
||
Comment on attachment 8738052 [details] [diff] [review] 0001-Bug-1260351-MozGlue-Build-Add-mips-support.-r-glandi.patch Review of attachment 8738052 [details] [diff] [review]: ----------------------------------------------------------------- ::: mozglue/build/mips.cpp @@ +19,5 @@ > +{ > + unsigned flags = 0; > + FILE *fin; > + > + fin = fopen ("/proc/cpuinfo","r"); no whitespace between function name and parenthesis. @@ +25,5 @@ > + char buf[1024]; > + memset(buf, 0, sizeof(buf)); > + fread(buf, sizeof(char), sizeof(buf) - 1, fin); > + fclose(fin); > + if (strstr(buf, "Loongson-3")) isn't any of the items in the "isa" field indicative of what you're looking for? That would be better than a processor name check, which will not work with future processors with a different name, but with the right instruction set. @@ +32,5 @@ > + return flags; > +} > + > +// Cache a local copy so we only have to read /proc/cpuinfo once. > +static unsigned mips_cpu_flags = get_mips_cpu_flags(); You could move this in the check function below.
Attachment #8738052 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #9) > Comment on attachment 8738052 [details] [diff] [review] > 0001-Bug-1260351-MozGlue-Build-Add-mips-support.-r-glandi.patch > > Review of attachment 8738052 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mozglue/build/mips.cpp > @@ +19,5 @@ > > +{ > > + unsigned flags = 0; > > + FILE *fin; > > + > > + fin = fopen ("/proc/cpuinfo","r"); > > no whitespace between function name and parenthesis. Nice! > > @@ +25,5 @@ > > + char buf[1024]; > > + memset(buf, 0, sizeof(buf)); > > + fread(buf, sizeof(char), sizeof(buf) - 1, fin); > > + fclose(fin); > > + if (strstr(buf, "Loongson-3")) > > isn't any of the items in the "isa" field indicative of what you're looking > for? That would be better than a processor name check, which will not work > with future processors with a different name, but with the right instruction > set. No. The Linux kernel doesn't export the MMI features in 'isa' and 'ASEs implemented' fields. As far as know, no one to do it. Fortunately, All Loongson-3 series CPUs are supported 'MMI' ASE. CPU info (Loongson-3): processor : 0 cpu model : ICT Loongson-3 V0.5 FPU V0.1 model name : ICT Loongson-3A R1 (Loongson-3A1000) @ 899MHz BogoMIPS : 717.28 wait instruction : no microsecond timers : yes tlb_entries : 64 extra interrupt vector : no hardware watchpoint : yes, count: 0, address/irw mask: [] isa : mips1 mips2 mips3 mips4 mips5 mips32r1 mips64r1 ASEs implemented : shadow register sets : 1 kscratch registers : 0 package : 0 core : 0 VCED exceptions : not available VCEI exceptions : not available > > @@ +32,5 @@ > > + return flags; > > +} > > + > > +// Cache a local copy so we only have to read /proc/cpuinfo once. > > +static unsigned mips_cpu_flags = get_mips_cpu_flags(); > > You could move this in the check function below. Did you mean move mips_cpu_flags in the check_loongson3 function below? A compiler error caused by use mips_cpu_flags before define it...
Assignee | ||
Comment 11•8 years ago
|
||
Attachment #8738839 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•8 years ago
|
Attachment #8738052 -
Attachment is obsolete: true
Comment 12•8 years ago
|
||
> Did you mean move mips_cpu_flags in the check_loongson3 function below? A compiler error caused by use mips_cpu_flags before define it...
How can the following fail to build with the error you say?
static bool
check_loongson3(void)
{
static unsigned mips_cpu_flags = get_mips_cpu_flags();
return (mips_cpu_flags & MIPS_FLAG_LOONGSON3) != 0;
}
Assignee | ||
Comment 13•8 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #12) > > Did you mean move mips_cpu_flags in the check_loongson3 function below? A compiler error caused by use mips_cpu_flags before define it... > > How can the following fail to build with the error you say? > > static bool > check_loongson3(void) > { > static unsigned mips_cpu_flags = get_mips_cpu_flags(); > return (mips_cpu_flags & MIPS_FLAG_LOONGSON3) != 0; > } sorry, I understand error :(
Assignee | ||
Comment 14•8 years ago
|
||
Attachment #8738839 -
Attachment is obsolete: true
Attachment #8738839 -
Flags: review?(mh+mozilla)
Attachment #8740237 -
Flags: review?(mh+mozilla)
Updated•8 years ago
|
Attachment #8740237 -
Flags: review?(mh+mozilla) → review+
Comment 15•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e11636c6aa2 https://hg.mozilla.org/integration/mozilla-inbound/rev/041577d97132
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6e11636c6aa2 https://hg.mozilla.org/mozilla-central/rev/041577d97132
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•