Bug 156338 – Modify the CXXFLAGS in webkitdirs.pm just on architectures where the flags are supported

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Tomas Popela]

When building from checkout on s390(x) or ppc(64) the build will fail as we are adding "-march=pentium4 -msse2 -mfpmath=sse " into CXXFLAGS in webkitdirs.pm. We need to avoid this step for these architectures.

[Tomas Popela]

Created attachment 275882 [details]
Proposed patch

[Michael Catanzaro]

Comment on attachment 275882 [details]
Proposed patch

I think it would be better to check for i[3,4,5,6]86 to decide whether to add those build flags, so that we don't need to grow this conditional for every imaginable arch.

[Michael Catanzaro]

Comment on attachment 275882 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=275882&action=review

> Tools/Scripts/webkitdirs.pm:2019
>          $ENV{'CXXFLAGS'} = "-march=pentium4 -msse2 -mfpmath=sse " . ($ENV{'CXXFLAGS'} || "");

Actually note that since it uses sse2, these flags only work on i686.

[Radar WebKit Bug Importer]

<rdar://problem/25599643>

[Tomas Popela]

(In reply to comment #3)
> Comment on attachment 275882 [details]
> Proposed patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=275882&action=review
> 
> > Tools/Scripts/webkitdirs.pm:2019
> >          $ENV{'CXXFLAGS'} = "-march=pentium4 -msse2 -mfpmath=sse " . ($ENV{'CXXFLAGS'} || "");
> 
> Actually note that since it uses sse2, these flags only work on i686.

So this would effectively leave us with:

-    if ($architecture ne "x86_64" && !isARM() && !isCrossCompilation() && !isAnyWindows()) {
+    if ($architecture eq "i686" && !isCrossCompilation() && !isAnyWindows()) {

but I don't know whether this could break anything for Apple port.

[Alex Christensen]

I don't think this will cause a problem for the Mac cmake work. If it causes a problem we will fix it, but my initial cmake work is only for x86_64

[Michael Catanzaro]

(In reply to comment #5)
> So this would effectively leave us with:
> 
> -    if ($architecture ne "x86_64" && !isARM() && !isCrossCompilation() &&
> !isAnyWindows()) {
> +    if ($architecture eq "i686" && !isCrossCompilation() &&
> !isAnyWindows()) {
> 
> but I don't know whether this could break anything for Apple port.

Let's indeed go with this. If it unexpectedly breaks something, we can simply revert it and use your original patch instead.

[Tomas Popela]

Created attachment 276225 [details]
Modify the CXXFLAGS just on i686

[WebKit Commit Bot]

Comment on attachment 276225 [details]
Modify the CXXFLAGS just on i686

Clearing flags on attachment: 276225

Committed r199350: <http://trac.webkit.org/changeset/199350>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.