Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2150943003:
Add MIPS SIMD Arch (MSA) build flags for GYP/GN builds (Closed)
|
Created:
4 years, 5 months ago by manojkumar.bhosale Modified:
4 years, 4 months ago CC:
chromium-reviews, tfarina, parag.salasakar_imgtec.com Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd MIPS SIMD Arch (MSA) build flags for GYP/GN builds
A preparation patch for adding MSA optimizations in libYUV project.
Committed: https://crrev.com/953e9fa0192b566ce280d895cbc3db2924c7e239
Cr-Commit-Position: refs/heads/master@{#413302}
Patch Set 1 #
Total comments: 3
Patch Set 2 : Add MIPS SIMD Arch (MSA) build flags for GYP/GN builds - Changes as per review comments #Patch Set 3 : Modified AUTHORs list to pass chromium_presubmit trybot #
Messages
Total messages: 54 (25 generated)
Description was changed from ========== Add MIPS SIMD Arch (MSA) build flags for GYP/GN builds R=thakis@chromium.org BUG= ========== to ========== Add MIPS SIMD Arch (MSA) build flags for GYP/GN builds A preparation patch for adding MSA optimizations in libYUV project. ==========
manojkumar.bhosale@imgtec.com changed reviewers: + justincohen@chromium.org, kjellander@chromium.org
Request to review the patch
dpranke@chromium.org changed reviewers: + dpranke@chromium.org
lgtm w/ the fixes, below, if you're sure you want things to default to true? https://codereview.chromium.org/2150943003/diff/1/build/config/mips.gni File build/config/mips.gni (right): https://codereview.chromium.org/2150943003/diff/1/build/config/mips.gni#newco... build/config/mips.gni:26: mips_use_msa = "true" it's mips_use_msa = true not mips_use_msa = "true" https://codereview.chromium.org/2150943003/diff/1/build/config/mips.gni#newco... build/config/mips.gni:48: mips_use_msa = "true" same https://codereview.chromium.org/2150943003/diff/1/tools/gn/docs/cookbook.md File tools/gn/docs/cookbook.md (right): https://codereview.chromium.org/2150943003/diff/1/tools/gn/docs/cookbook.md#n... tools/gn/docs/cookbook.md:227: | `mips_msa` (0/1) | `mips_use_msa` (true/false) | `//build/config/mips.gni` | I wouldn't add this change; we don't list every flag, just the most common ones.
On 2016/07/15 18:59:59, Dirk Pranke wrote: > lgtm w/ the fixes, below, if you're sure you want things to default to true? > > https://codereview.chromium.org/2150943003/diff/1/build/config/mips.gni > File build/config/mips.gni (right): > > https://codereview.chromium.org/2150943003/diff/1/build/config/mips.gni#newco... > build/config/mips.gni:26: mips_use_msa = "true" > it's > > mips_use_msa = true > > not > > mips_use_msa = "true" > > https://codereview.chromium.org/2150943003/diff/1/build/config/mips.gni#newco... > build/config/mips.gni:48: mips_use_msa = "true" > same > > https://codereview.chromium.org/2150943003/diff/1/tools/gn/docs/cookbook.md > File tools/gn/docs/cookbook.md (right): > > https://codereview.chromium.org/2150943003/diff/1/tools/gn/docs/cookbook.md#n... > tools/gn/docs/cookbook.md:227: | `mips_msa` (0/1) > | `mips_use_msa` (true/false) | `//build/config/mips.gni` > | > I wouldn't add this change; we don't list every flag, just the most common ones. Yes, OK with default mips_use_msa = true & other comments Thanks
Changes as per review comments
Hi, May I request to review the patch and provide further comments, if any. Thanks,
still lgtm.
On 2016/07/25 20:20:01, Dirk Pranke wrote: > still lgtm. Hi, May I request a further action on this? Thanks, Manoj
On 2016/08/04 07:50:39, manojkumar.bhosale wrote: > On 2016/07/25 20:20:01, Dirk Pranke wrote: > > still lgtm. > > Hi, > May I request a further action on this? > > Thanks, > Manoj Once you have an "lgtm" from the reviewer, you can just click the "Commit" box to land the change. (Which I've just done for you).
The CQ bit was checked by dpranke@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
The author manojkumar.bhosale@imgtec.com has not signed Google Contributor License Agreement. Please visit https://cla.developers.google.com to sign and manage CLA.
Hi, It seems the change is not landed because of CLA issue. Imagination Technologies Ltd has already signed CCLA attached herein. Is this sufficient to add me as a contributor? Thanks, Manoj From: commit-bot@chromium.org via codereview.chromium.org [mailto:reply@chromiumcodereview-hr.appspotmail.com] Sent: Thursday, August 4, 2016 9:21 PM To: Manojkumar Bhosale; thakis@chromium.org; kjellander@chromium.org; justincohen@chromium.org; dpranke@chromium.org; commit-bot@chromium.org Cc: chromium-reviews@chromium.org; tfarina@chromium.org; Parag Salasakar Subject: Re: Add MIPS SIMD Arch (MSA) build flags for GYP/GN builds (issue 2150943003 by manojkumar.bhosale@imgtec.com) The author manojkumar.bhosale@imgtec.com<mailto:manojkumar.bhosale@imgtec.com> has not signed Google Contributor License Agreement. Please visit https://cla.developers.google.com to sign and manage CLA. https://codereview.chromium.org/2150943003/ -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Yes, that should be sufficient, and I'm not sure why the commit check failed. We'll get someone to look into it. -- Dirk On Mon, Aug 8, 2016 at 11:54 PM, Manojkumar Bhosale < Manojkumar.Bhosale@imgtec.com> wrote: > Hi, > > It seems the change is not landed because of CLA issue. > > > > Imagination Technologies Ltd has already signed CCLA attached herein. > > Is this sufficient to add me as a contributor? > > > > Thanks, > > Manoj > > > > *From:* commit-bot@chromium.org via codereview.chromium.org [mailto:reply@ > chromiumcodereview-hr.appspotmail.com] > *Sent:* Thursday, August 4, 2016 9:21 PM > *To:* Manojkumar Bhosale; thakis@chromium.org; kjellander@chromium.org; > justincohen@chromium.org; dpranke@chromium.org; commit-bot@chromium.org > *Cc:* chromium-reviews@chromium.org; tfarina@chromium.org; Parag Salasakar > *Subject:* Re: Add MIPS SIMD Arch (MSA) build flags for GYP/GN builds > (issue 2150943003 by manojkumar.bhosale@imgtec.com) > > > > The author manojkumar.bhosale@imgtec.com has not signed Google Contributor > License Agreement. Please visit https://cla.developers.google.com to sign > and > manage CLA. > > https://codereview.chromium.org/2150943003/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
The CQ bit was checked by dpranke@chromium.org
I filed bug 636046 to follow up on this. I checked the commit box again, just to see if this was a transient failure. Assuming we're not in a huge hurry for this, I'd rather wait to fix the issue and get this landed through the normal processes rather than try to bypass them, if that's okay.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
The author manojkumar.bhosale@imgtec.com has not signed Google Contributor License Agreement. Please visit https://cla.developers.google.com to sign and manage CLA.
The CQ bit was checked by manojkumar.bhosale@imgtec.com
The CQ bit was unchecked by manojkumar.bhosale@imgtec.com
The CQ bit was checked by manojkumar.bhosale@imgtec.com
The CQ bit was unchecked by manojkumar.bhosale@imgtec.com
The CQ bit was checked by manojkumar.bhosale@imgtec.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
The author manojkumar.bhosale@imgtec.com has not signed Google Contributor License Agreement. Please visit https://cla.developers.google.com to sign and manage CLA.
The CQ bit was checked by manojkumar.bhosale@imgtec.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
The author manojkumar.bhosale@imgtec.com has not signed Google Contributor License Agreement. Please visit https://cla.developers.google.com to sign and manage CLA.
The CQ bit was checked by manojkumar.bhosale@imgtec.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
The CQ bit was checked by manojkumar.bhosale@imgtec.com
The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org Link to the patchset: https://codereview.chromium.org/2150943003/#ps40001 (title: "Modified AUTHORs list to pass chromium_presubmit trybot")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by dpranke@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by dpranke@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add MIPS SIMD Arch (MSA) build flags for GYP/GN builds A preparation patch for adding MSA optimizations in libYUV project. ========== to ========== Add MIPS SIMD Arch (MSA) build flags for GYP/GN builds A preparation patch for adding MSA optimizations in libYUV project. ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Add MIPS SIMD Arch (MSA) build flags for GYP/GN builds A preparation patch for adding MSA optimizations in libYUV project. ========== to ========== Add MIPS SIMD Arch (MSA) build flags for GYP/GN builds A preparation patch for adding MSA optimizations in libYUV project. Committed: https://crrev.com/953e9fa0192b566ce280d895cbc3db2924c7e239 Cr-Commit-Position: refs/heads/master@{#413302} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/953e9fa0192b566ce280d895cbc3db2924c7e239 Cr-Commit-Position: refs/heads/master@{#413302} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
