Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1867193002:
Add AVX/AVX2 support for runtime check (Closed)
DescriptionAdd AVX/AVX2 support for runtime check
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1867193002
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
Committed: https://skia.googlesource.com/skia/+/601e36a5c5858d1940a8c381f832e2867acbfb8e
Patch Set 1 #Patch Set 2 : Remove redundant getcpuid functions #Patch Set 3 : Add DEPRECATED disclaimer #Patch Set 4 : Add Sergey Melnikov to AUTHORS #Messages
Total messages: 23 (9 generated)
Description was changed from ========== Add AVX/AVX2 support for runtime check BUG=skia: ========== to ========== Add AVX/AVX2 support for runtime check BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot ==========
Description was changed from ========== Add AVX/AVX2 support for runtime check BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot ========== to ========== Add AVX/AVX2 support for runtime check BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot ==========
melnikov.sergey.v@gmail.com changed reviewers: + mtklein@google.com
Hi, Could you take a loop at my patch? Patch allows to check AVX/AVX2 availability at run-time.
On 2016/04/08 at 00:26:33, Melnikov.Sergey.V wrote: > Hi, > > Could you take a loop at my patch? > Patch allows to check AVX/AVX2 availability at run-time. We're generally doing new CPU-specific work in the SkOpts framework now; we've just been slow to port old code over. Want to take a look at the AVX and AVX2 detection code in src/core/SkOpts.cpp to see if it looks right to you? They're currently detected but not put to much use.
On 2016/04/08 00:50:07, mtklein wrote: > On 2016/04/08 at 00:26:33, Melnikov.Sergey.V wrote: > > Hi, > > > > Could you take a loop at my patch? > > Patch allows to check AVX/AVX2 availability at run-time. > > We're generally doing new CPU-specific work in the SkOpts framework now; we've > just been slow to port old code over. > > Want to take a look at the AVX and AVX2 detection code in src/core/SkOpts.cpp to > see if it looks right to you? They're currently detected but not put to much > use. Thanks, that code looks good. So, should I cancel my review? May be we should add a kind of 'deprecated' comment to this file (opts_check_x86)?
On 2016/04/08 at 08:33:03, Melnikov.Sergey.V wrote: > On 2016/04/08 00:50:07, mtklein wrote: > > On 2016/04/08 at 00:26:33, Melnikov.Sergey.V wrote: > > > Hi, > > > > > > Could you take a loop at my patch? > > > Patch allows to check AVX/AVX2 availability at run-time. > > > > We're generally doing new CPU-specific work in the SkOpts framework now; we've > > just been slow to port old code over. > > > > Want to take a look at the AVX and AVX2 detection code in src/core/SkOpts.cpp to > > see if it looks right to you? They're currently detected but not put to much > > use. > > Thanks, that code looks good. So, should I cancel my review? May be we should add a kind of 'deprecated' comment to this file (opts_check_x86)? Sure, yeah, that's a good idea.
On 2016/04/08 11:48:48, mtklein wrote: > On 2016/04/08 at 08:33:03, Melnikov.Sergey.V wrote: > > On 2016/04/08 00:50:07, mtklein wrote: > > > On 2016/04/08 at 00:26:33, Melnikov.Sergey.V wrote: > > > > Hi, > > > > > > > > Could you take a loop at my patch? > > > > Patch allows to check AVX/AVX2 availability at run-time. > > > > > > We're generally doing new CPU-specific work in the SkOpts framework now; > we've > > > just been slow to port old code over. > > > > > > Want to take a look at the AVX and AVX2 detection code in > src/core/SkOpts.cpp to > > > see if it looks right to you? They're currently detected but not put to > much > > > use. > > > > Thanks, that code looks good. So, should I cancel my review? May be we should > add a kind of 'deprecated' comment to this file (opts_check_x86)? > > Sure, yeah, that's a good idea. I've added a note to the beginning of file.
The CQ bit was checked by mtklein@google.com
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1867193002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1867193002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED, http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)
On 2016/04/08 at 19:08:18, commit-bot wrote: > Try jobs failed on following builders: > skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED, http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...) ** Presubmit ERRORS ** The email Melnikov.Sergey.V@gmail.com is not in Skia's AUTHORS file. Issue owner, this CL must include an addition to the Skia AUTHORS file.
On 2016/04/08 19:09:28, mtklein wrote: > On 2016/04/08 at 19:08:18, commit-bot wrote: > > Try jobs failed on following builders: > > skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED, > http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...) > > ** Presubmit ERRORS ** > The email mailto:Melnikov.Sergey.V@gmail.com is not in Skia's AUTHORS file. > Issue owner, this CL must include an addition to the Skia AUTHORS file. I've added myself to AUTHORS
mtklein@chromium.org changed reviewers: + mtklein@chromium.org
The CQ bit was checked by mtklein@chromium.org
lgtm
The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@google.com Link to the patchset: https://codereview.chromium.org/1867193002/#ps60001 (title: "Add Sergey Melnikov to AUTHORS")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1867193002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1867193002/60001
Message was sent while issue was closed.
Description was changed from ========== Add AVX/AVX2 support for runtime check BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot ========== to ========== Add AVX/AVX2 support for runtime check BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/601e36a5c5858d1940a8c381f832e2867acbfb8e ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://skia.googlesource.com/skia/+/601e36a5c5858d1940a8c381f832e2867acbfb8e
Message was sent while issue was closed.
On 2016/04/08 22:31:48, commit-bot: I haz the power wrote: > Committed patchset #4 (id:60001) as > https://skia.googlesource.com/skia/+/601e36a5c5858d1940a8c381f832e2867acbfb8e Thanks, mtklein! |
