Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2244983002:
-fno-auto-profile is only available beginning with stock GCC 5 (Closed)
|
Created:
4 years, 4 months ago by Mostyn Bramley-Moore Modified:
4 years, 4 months ago CC:
chromium-reviews, wfh+watch_chromium.org, Dai Mikurube (NOT FULLTIME) Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description-fno-auto-profile is only available beginning with stock GCC 5
This followup to https://codereview.chromium.org/2198253002
unbreaks stock GCC 4.8/4.9 builds.
(We suspect that the chromeos GCC 4.9 toolchain has a local patch for this feature.)
BUG=629593
Committed: https://crrev.com/2af5a28d83c844c1f58aacb59a9b8b1cb234b129
Cr-Commit-Position: refs/heads/master@{#412267}
Patch Set 1 #Patch Set 2 : add chromeos logic #Messages
Total messages: 31 (15 generated)
The CQ bit was checked by mostynb@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
mostynb@opera.com changed reviewers: + llozano@chromium.org, primiano@chromium.org
@primiano, llozano: please take a look at this followup to https://codereview.chromium.org/2198253002 - this unbreaks GCC < 5 builds.
Description was changed from ========== -fno-auto-profile is only available beginning with GCC 5 BUG=629593 ========== to ========== -fno-auto-profile is only available beginning with GCC 5 This followup to https://codereview.chromium.org/2198253002 unbreaks GCC 4.8/4.9 builds. BUG=629593 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/08/14 23:08:32, Mostyn Bramley-Moore wrote: > @primiano, llozano: please take a look at this followup to > https://codereview.chromium.org/2198253002 - this unbreaks GCC < 5 builds. The condition is incorrect. I used this with the 4.9 GCC compiler. ChromeOS uses 4.9. #if ((__GNUC__ == 4 && __GNUC_MINOR__ >= 9) || __GNUC__ > 4) https://codereview.chromium.org/2198253002#msg21 had the version of the fix that checked for 4.9 but I was told to use #if !defined(__clang__) instead.
On 2016/08/15 06:52:57, llozano wrote: > On 2016/08/14 23:08:32, Mostyn Bramley-Moore wrote: > > @primiano, llozano: please take a look at this followup to > > https://codereview.chromium.org/2198253002 - this unbreaks GCC < 5 builds. > > The condition is incorrect. > I used this with the 4.9 GCC compiler. > ChromeOS uses 4.9. > > #if ((__GNUC__ == 4 && __GNUC_MINOR__ >= 9) || __GNUC__ > 4) > > https://codereview.chromium.org/2198253002#msg21 had the version of the fix that > checked for 4.9 but I was told to use #if !defined(__clang__) > instead. The version of GCC 4.9 available in Ubuntu does not support this option: $ touch empty.cc $ g++-4.8 -fno-auto-profile -c empty.cc g++-4.8: error: unrecognized command line option ‘-fno-auto-profile’ $ g++-4.9 -fno-auto-profile -c empty.cc g++-4.9: error: unrecognized command line option ‘-fno-auto-profile’ $ g++-5 -fno-auto-profile -c empty.cc Can you try this test with the chromeos compiler?
On 2016/08/15 07:19:42, Mostyn Bramley-Moore wrote: > On 2016/08/15 06:52:57, llozano wrote: > > On 2016/08/14 23:08:32, Mostyn Bramley-Moore wrote: > > > @primiano, llozano: please take a look at this followup to > > > https://codereview.chromium.org/2198253002 - this unbreaks GCC < 5 builds. > > > > The condition is incorrect. > > I used this with the 4.9 GCC compiler. > > ChromeOS uses 4.9. > > > > #if ((__GNUC__ == 4 && __GNUC_MINOR__ >= 9) || __GNUC__ > 4) > > > > https://codereview.chromium.org/2198253002#msg21 had the version of the fix > that > > checked for 4.9 but I was told to use #if !defined(__clang__) > > instead. > > The version of GCC 4.9 available in Ubuntu does not support this option: > > $ touch empty.cc > $ g++-4.8 -fno-auto-profile -c empty.cc > g++-4.8: error: unrecognized command line option ‘-fno-auto-profile’ > $ g++-4.9 -fno-auto-profile -c empty.cc > g++-4.9: error: unrecognized command line option ‘-fno-auto-profile’ > $ g++-5 -fno-auto-profile -c empty.cc > > Can you try this test with the chromeos compiler? this clearly works on ChromeOS (4.9.2). We have been building with this for over a week. But I am not sure if there are some changes in the google branch that only made it upstream in GCC 5.0. I will figure out tomorrow. There is a different fix coming (will get rid of the pragma optimize). Can you wait a few days for the other fix?
> > $ touch empty.cc > > $ g++-4.8 -fno-auto-profile -c empty.cc > > g++-4.8: error: unrecognized command line option ‘-fno-auto-profile’ > > $ g++-4.9 -fno-auto-profile -c empty.cc > > g++-4.9: error: unrecognized command line option ‘-fno-auto-profile’ > > $ g++-5 -fno-auto-profile -c empty.cc > > > > Can you try this test with the chromeos compiler? > > this clearly works on ChromeOS (4.9.2). We have been building with this for over > a week. > But I am not sure if there are some changes in the google branch that only made > it upstream in GCC 5.0. I will figure out tomorrow. > There is a different fix coming (will get rid of the pragma optimize). Can you > wait a few days for the other fix? I can wait a couple more days, if there's no good fix available now. If it's going to take a bit longer, then perhaps we can add some chromeos logic to this #if block?
On 2016/08/15 07:59:06, Mostyn Bramley-Moore wrote: > > > $ touch empty.cc > > > $ g++-4.8 -fno-auto-profile -c empty.cc > > > g++-4.8: error: unrecognized command line option ‘-fno-auto-profile’ > > > $ g++-4.9 -fno-auto-profile -c empty.cc > > > g++-4.9: error: unrecognized command line option ‘-fno-auto-profile’ > > > $ g++-5 -fno-auto-profile -c empty.cc > > > > > > Can you try this test with the chromeos compiler? > > > > this clearly works on ChromeOS (4.9.2). We have been building with this for > over > > a week. > > But I am not sure if there are some changes in the google branch that only > made > > it upstream in GCC 5.0. I will figure out tomorrow. > > There is a different fix coming (will get rid of the pragma optimize). Can you > > wait a few days for the other fix? > > I can wait a couple more days, if there's no good fix available now. If it's > going to take a bit longer, then perhaps we can add some chromeos logic to this > #if block? yes, that is an option. Unfortunately there is not general __CHROMEOS__ define right now.
> > I can wait a couple more days, if there's no good fix available now. If it's > > going to take a bit longer, then perhaps we can add some chromeos logic to > this > > #if block? > > yes, that is an option. Unfortunately there is not general __CHROMEOS__ define > right now. Would OS_CHROMEOS work?
On 2016/08/15 13:39:51, Mostyn Bramley-Moore wrote: > > > I can wait a couple more days, if there's no good fix available now. If > it's > > > going to take a bit longer, then perhaps we can add some chromeos logic to > > this > > > #if block? > > > > yes, that is an option. Unfortunately there is not general __CHROMEOS__ define > > right now. > > Would OS_CHROMEOS work? Yes, that should work. I was not sure this was defined in all the ChromeOS workflows but it is. Since you already have the CL, would you mind doing the change and I can review?
The CQ bit was checked by mostynb@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
How does patchset 2 look?
lgtm
On 2016/08/16 08:09:43, llozano wrote: > lgtm thanks
mostynb@opera.com changed reviewers: + wfh@chromium.org - primiano@chromium.org
No problem. @Will: can you please give this OWNERS approval?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== -fno-auto-profile is only available beginning with GCC 5 This followup to https://codereview.chromium.org/2198253002 unbreaks GCC 4.8/4.9 builds. BUG=629593 ========== to ========== -fno-auto-profile is only available beginning with stock GCC 5 This followup to https://codereview.chromium.org/2198253002 unbreaks stock GCC 4.8/4.9 builds. (We suspect that the chromeos GCC 4.9 toolchain has a local patch for this feature.) BUG=629593 ==========
lgtm
The CQ bit was checked by mostynb@opera.com
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 ========== -fno-auto-profile is only available beginning with stock GCC 5 This followup to https://codereview.chromium.org/2198253002 unbreaks stock GCC 4.8/4.9 builds. (We suspect that the chromeos GCC 4.9 toolchain has a local patch for this feature.) BUG=629593 ========== to ========== -fno-auto-profile is only available beginning with stock GCC 5 This followup to https://codereview.chromium.org/2198253002 unbreaks stock GCC 4.8/4.9 builds. (We suspect that the chromeos GCC 4.9 toolchain has a local patch for this feature.) BUG=629593 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== -fno-auto-profile is only available beginning with stock GCC 5 This followup to https://codereview.chromium.org/2198253002 unbreaks stock GCC 4.8/4.9 builds. (We suspect that the chromeos GCC 4.9 toolchain has a local patch for this feature.) BUG=629593 ========== to ========== -fno-auto-profile is only available beginning with stock GCC 5 This followup to https://codereview.chromium.org/2198253002 unbreaks stock GCC 4.8/4.9 builds. (We suspect that the chromeos GCC 4.9 toolchain has a local patch for this feature.) BUG=629593 Committed: https://crrev.com/2af5a28d83c844c1f58aacb59a9b8b1cb234b129 Cr-Commit-Position: refs/heads/master@{#412267} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/2af5a28d83c844c1f58aacb59a9b8b1cb234b129 Cr-Commit-Position: refs/heads/master@{#412267} |
