Disable auto-vectorization of P224 ReduceLarge() under GCC

Disable auto-vectorization of P224 ReduceLarge() under GCC.

Something about automatic tree vectorization breaks ReduceLarge(), so
disable the optimization for this function until we determine the real
fix.

BUG=439566
Committed: https://crrev.com/af9ea6f018c635e4cfadebdc7b6a0715d44441aa
Cr-Commit-Position: refs/heads/master@{#309335}

[Wez, 2014-12-19 19:55:32]

wez@chromium.org changed reviewers:
+ agl@chromium.org, cmtice@chromium.org

[Wez, 2014-12-19 19:55:33]

cmtice: Please verify that I've added the pragmas as you intended!
agl: Please provide OWNERS approval.

[agl, 2014-12-19 20:04:50]

LGTM

However, please see trybot results – looks like some ifdefs are needed because
unrecognised pragmas are resulting in errors.

[chromium-reviews, 2014-12-19 20:10:43]

Can you send me a link for a failing trybot?

-- Caroline Tice
cmtice@google.com

On Fri, Dec 19, 2014 at 12:04 PM,  <agl@chromium.org> wrote:
> LGTM
>
> However, please see trybot results – looks like some ifdefs are needed
> because
> unrecognised pragmas are resulting in errors.
>
> https://codereview.chromium.org/814273004/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[cmtice1, 2014-12-19 20:12:13]

On 2014/12/19 19:55:33, Wez wrote:
> cmtice: Please verify that I've added the pragmas as you intended!
> agl: Please provide OWNERS approval.

The pragmas look correct to me.

[agl, 2014-12-19 20:17:10]

On Fri, Dec 19, 2014 at 12:10 PM, Caroline Tice <cmtice@google.com> wrote:
> Can you send me a link for a failing trybot?

http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...


Cheers

AGL

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[cmtice1, 2014-12-19 20:24:23]

On 2014/12/19 20:04:50, agl wrote:
> LGTM
> 
> However, please see trybot results – looks like some ifdefs are needed because
> unrecognised pragmas are resulting in errors.

It looks like clang is complaining about the GCC pragmas.  You might try either
using -Wno-unknown-pragma, which I believe will disable warnings for pragmas it
doesn't recognize. Or you could try using a #if defined __GNUC__, to try to have
the pragmas looked at by GCC and not by clang.

[cmtice1, 2014-12-19 20:29:19]

On 2014/12/19 20:24:23, cmtice1 wrote:
> On 2014/12/19 20:04:50, agl wrote:
> > LGTM
> > 
> > However, please see trybot results – looks like some ifdefs are needed
because
> > unrecognised pragmas are resulting in errors.
> 
> It looks like clang is complaining about the GCC pragmas.  You might try
either
> using -Wno-unknown-pragma, which I believe will disable warnings for pragmas
it
> doesn't recognize. Or you could try using a #if defined __GNUC__, to try to
have
> the pragmas looked at by GCC and not by clang.

Here's a better example of using an ifdef on a GCC pragma (from
src/url/url_cannon_ip.cc, line 208, modified slightly):

#if ((__GNUC__ == 4 && __GNUC_MINOR >= 8) || __GNUC__ > 4)
#pragma GCC etc.
#endif

[Wez, 2014-12-19 21:05:43]

On 2014/12/19 20:29:19, cmtice1 wrote:
> On 2014/12/19 20:24:23, cmtice1 wrote:
> > On 2014/12/19 20:04:50, agl wrote:
> > > LGTM
> > > 
> > > However, please see trybot results – looks like some ifdefs are needed
> because
> > > unrecognised pragmas are resulting in errors.
> > 
> > It looks like clang is complaining about the GCC pragmas.  You might try
> either
> > using -Wno-unknown-pragma, which I believe will disable warnings for pragmas
> it
> > doesn't recognize. Or you could try using a #if defined __GNUC__, to try to
> have
> > the pragmas looked at by GCC and not by clang.
> 
> Here's a better example of using an ifdef on a GCC pragma (from
> src/url/url_cannon_ip.cc, line 208, modified slightly):
> 
> #if ((__GNUC__ == 4 && __GNUC_MINOR >= 8) || __GNUC__ > 4)
> #pragma GCC etc.
> #endif

Do we really need the version check? Not sure what GCC versions Chrome is
intended to be buildable with.

[cmtice1, 2014-12-19 21:08:19]

On 2014/12/19 21:05:43, Wez wrote:
> On 2014/12/19 20:29:19, cmtice1 wrote:
> > On 2014/12/19 20:24:23, cmtice1 wrote:
> > > On 2014/12/19 20:04:50, agl wrote:
> > > > LGTM
> > > > 
> > > > However, please see trybot results – looks like some ifdefs are needed
> > because
> > > > unrecognised pragmas are resulting in errors.
> > > 
> > > It looks like clang is complaining about the GCC pragmas.  You might try
> > either
> > > using -Wno-unknown-pragma, which I believe will disable warnings for
pragmas
> > it
> > > doesn't recognize. Or you could try using a #if defined __GNUC__, to try
to
> > have
> > > the pragmas looked at by GCC and not by clang.
> > 
> > Here's a better example of using an ifdef on a GCC pragma (from
> > src/url/url_cannon_ip.cc, line 208, modified slightly):
> > 
> > #if ((__GNUC__ == 4 && __GNUC_MINOR >= 8) || __GNUC__ > 4)
> > #pragma GCC etc.
> > #endif
> 
> Do we really need the version check? Not sure what GCC versions Chrome is
> intended to be buildable with.

We don't need a particular version, that's just the best example I could find of
how to do this.  The check I showed is checking for gcc 4.8or higher, but you
could reduce that even further if you wished...

[yunlian, 2014-12-19 23:21:56]

On 2014/12/19 21:08:19, cmtice1 wrote:
> On 2014/12/19 21:05:43, Wez wrote:
> > On 2014/12/19 20:29:19, cmtice1 wrote:
> > > On 2014/12/19 20:24:23, cmtice1 wrote:
> > > > On 2014/12/19 20:04:50, agl wrote:
> > > > > LGTM
> > > > > 
> > > > > However, please see trybot results – looks like some ifdefs are needed
> > > because
> > > > > unrecognised pragmas are resulting in errors.
> > > > 
> > > > It looks like clang is complaining about the GCC pragmas.  You might try
> > > either
> > > > using -Wno-unknown-pragma, which I believe will disable warnings for
> pragmas
> > > it
> > > > doesn't recognize. Or you could try using a #if defined __GNUC__, to try
> to
> > > have
> > > > the pragmas looked at by GCC and not by clang.
> > > 
> > > Here's a better example of using an ifdef on a GCC pragma (from
> > > src/url/url_cannon_ip.cc, line 208, modified slightly):
> > > 
> > > #if ((__GNUC__ == 4 && __GNUC_MINOR >= 8) || __GNUC__ > 4)
> > > #pragma GCC etc.
> > > #endif
> > 
> > Do we really need the version check? Not sure what GCC versions Chrome is
> > intended to be buildable with.
> 
> We don't need a particular version, that's just the best example I could find
of
> how to do this.  The check I showed is checking for gcc 4.8or higher, but you
> could reduce that even further if you wished...

Or we could use
#ifudef __clang__
#pragma ...
#endif

[Wez, 2014-12-20 00:24:38]

Is lack of understanding of #pragma GCC ... specific to clang? I'm
surprised that clang would be broken by it but e.g. MSVC not.

On 19 December 2014 at 15:21, <yunlian@chromium.org> wrote:

> On 2014/12/19 21:08:19, cmtice1 wrote:
>
>> On 2014/12/19 21:05:43, Wez wrote:
>> > On 2014/12/19 20:29:19, cmtice1 wrote:
>> > > On 2014/12/19 20:24:23, cmtice1 wrote:
>> > > > On 2014/12/19 20:04:50, agl wrote:
>> > > > > LGTM
>> > > > >
>> > > > > However, please see trybot results – looks like some ifdefs are
>> needed
>> > > because
>> > > > > unrecognised pragmas are resulting in errors.
>> > > >
>> > > > It looks like clang is complaining about the GCC pragmas.  You
>> might try
>> > > either
>> > > > using -Wno-unknown-pragma, which I believe will disable warnings for
>> pragmas
>> > > it
>> > > > doesn't recognize. Or you could try using a #if defined __GNUC__,
>> to try
>> to
>> > > have
>> > > > the pragmas looked at by GCC and not by clang.
>> > >
>> > > Here's a better example of using an ifdef on a GCC pragma (from
>> > > src/url/url_cannon_ip.cc, line 208, modified slightly):
>> > >
>> > > #if ((__GNUC__ == 4 && __GNUC_MINOR >= 8) || __GNUC__ > 4)
>> > > #pragma GCC etc.
>> > > #endif
>> >
>> > Do we really need the version check? Not sure what GCC versions Chrome
>> is
>> > intended to be buildable with.
>>
>
>  We don't need a particular version, that's just the best example I could
>> find
>>
> of
>
>> how to do this.  The check I showed is checking for gcc 4.8or higher, but
>> you
>> could reduce that even further if you wished...
>>
>
> Or we could use
> #ifudef __clang__
> #pragma ...
> #endif
>
> https://codereview.chromium.org/814273004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Wez, 2014-12-20 03:32:56]

The CQ bit was checked by wez@chromium.org

[commit-bot: I haz the power, 2014-12-20 03:33:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/814273004/60001

[commit-bot: I haz the power, 2014-12-20 04:30:04]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-12-20 04:30:42]

Patchset 4 (id:??) landed as
https://crrev.com/af9ea6f018c635e4cfadebdc7b6a0715d44441aa
Cr-Commit-Position: refs/heads/master@{#309335}