372878 - reduce the number of AC_TRY_RUN tests in configure.in

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

We should reduce the number of AC_TRY_RUN tests we have in configure.in, since such tests don't work when cross-compiling.  Missing some of them causes us to miss out on some rather important performance optimizations when cross-compiling.  (And we actually use cross-compilation for Mac universal, among other things.)

Many of the current AC_TRY_RUN tests in configure could be written using AC_TRY_COMPILE.

(I got started on this after discussing bug 257197 comment 84 with andrew, when I pointed out that the AC_TRY_RUN test after --enable-system-png could be an AC_TRY_COMPILE test with #error instead of exit().)

Some tests that could be converted include:

 * tests for sizeof() could be rewritten to depend on array size typedefs requiring positive arguments (or is it nonnegative?  hopefully that's portable.  We could, in fact, check that it's portable before doing it, by rejecting compilers where the test strategy doesn't work)

 * the va_copy tests look like they could just be AC_TRY_COMPILE tests, since they always return 0 (unless they're testing for segfaults)

 * the SYSTEM_JPEG and SYSTEM_PNG version tests could be rewritten using #error rather than exit()

 * the zlib version test could be written using ZLIB_VERNUM instead of ZLIB_VERSION, and thus using #error

That leaves only:
 * the test for MMAP_MISSES_WRITES (already OK, with pessimistic default)
 * the test for dynamic_cast<void*>() (which is for debugging code only)
 * the test for correct temporary object destruction order (already basically OK; we just miss the test)
 * code in a fallback case for libIDL config program not found (already OK)

Benjamin -- I'm not sure how this interacts with your rewrite-in-python plans, but I suspect you're going to want to keep a lot of the existing tests anyway.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #0)
>  * the va_copy tests look like they could just be AC_TRY_COMPILE tests, since
> they always return 0 (unless they're testing for segfaults)

For what it's worth, these were added in bug 187180.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: add static assertion macros and convert just the sizeof tests

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: add static assertion macros and convert just the sizeof tests

Oops, forgot to bump the cache variable names.

Note that I'd also like to get this patch (but not other patches needed to finish fixing this bug) in on the 1.8 branch, since it's what would get the Mac universal build's cross-compiled half back to using the fast version of NS_LITERAL_STRING/NS_NAMED_LITERAL_STRING, which IIRC is a measurable performance difference.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

attachment 257567 [details] [diff] [review] checked in to trunk.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 257567 [details] [diff] [review]
add static assertion macros and convert just the sizeof tests

I'd like to land this on the 1.8 branch because it prevents a bunch of pretty important performance optimizations (in particular, the fast version of NS_LITERAL_STRING) from being used in the cross-compiled half of Mac universal builds.  I think it's pretty low risk, at least for our own builds, and I wouldn't be surprised if it helps some Linux distros too.

Comment 6

[Benjamin Smedberg]

I think we should let this bake a couple weeks before landing on the branch.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: convert system library version tests

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And along with that I should bump the libpng version to what we actually use; bug 334110 forgot to do that.

Comment 9

[(not currently active) Ted Mielczarek]

Comment on attachment 257891 [details] [diff] [review]
convert system library version tests

Just two drive-by comments:

+                   [ #if $MOZZLIB > ZLIB_VERNUM
You should probably reverse that comparison to match the others.

Also, can you put the expected minimum version number in the #error message?

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment 257891 [details] [diff], plus improvements from comment 9 (but not comment 8!) checked in to trunk.  That said, it's sort of irrelevant now that we require APNG functions to be present.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 257567 [details] [diff] [review]
add static assertion macros and convert just the sizeof tests

approved for 1.8.1.4, a=dveditz for release-drivers

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment 257567 [details] [diff] to MOZILLA_1_8_BRANCH.  Adding fixed1.8.1.4 keyword since that's the only patch I want to land on the branch (at this point, anyway).

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I checked the logs of "MacOSX Darwin 8.7.0 bm-xserve02 Depend Fx-Nightly" on the Mozilla1.8 tinderbox, and landing this patch changed, in the second half of the universal build, this (old):

checking for usable wchar_t (2 bytes, unsigned)... maybe
checking for compiler -fshort-wchar option... maybe

into this (new):

checking for usable wchar_t (2 bytes, unsigned)... no
checking for compiler -fshort-wchar option... yes

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And since I'd rather not touch the va_copy tests, since I'm not sure what they're really testing for, I'm going to mark this fixed for the trunk as well.

Comment 15

[Doug Turner (:dougt)]

This seams to have broken the windows mobile port:

configure: error: Compiler cannot compile macros used in autoconf tests.
*** Fix above errors and then restart with "make -f client.mk build"
make[1]: *** [configure] Error 1

Any information to aide in debugging, please let me know.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The errors in config.log ?

Comment 17

[Doug Turner (:dougt)]

configure:5365: checking that static assertion macros used in autoconf tests work
configure:5386: arm-wince-gcc -c  -TC -nologo  conftest.c 1>&5
c:/Program Files/Microsoft Visual Studio 8/VC/ce/bin/x86_arm/cl.exe /I"c:/Program Files/Microsoft Visual Studio 8/SmartDevices/SDK/PocketPC2003/Include" /I"x:/builds/minimo/mozilla/build/wince/shunt/include/" /FI"mozce_shunt.h" /DMOZCE_STATIC_BUILD /D_WIN32_WCE=420 /DUNDER_CE=420 /DWIN32_PLATFORM_PSPC=400 /DARM /DWINCE /D_ARM_ /Zc:wchar_t- /GS- /GR- -c -TC -nologo conftest.c 

conftest.c

configure:5403: arm-wince-gcc -c  -TC -nologo  conftest.c 1>&5
c:/Program Files/Microsoft Visual Studio 8/VC/ce/bin/x86_arm/cl.exe /I"c:/Program Files/Microsoft Visual Studio 8/SmartDevices/SDK/PocketPC2003/Include" /I"x:/builds/minimo/mozilla/build/wince/shunt/include/" /FI"mozce_shunt.h" /DMOZCE_STATIC_BUILD /D_WIN32_WCE=420 /DUNDER_CE=420 /DWIN32_PLATFORM_PSPC=400 /DARM /DWINCE /D_ARM_ /Zc:wchar_t- /GS- /GR- -c -TC -nologo conftest.c 

conftest.c

configure(5403) : error C2118: negative subscript

configure:5426: arm-wince-gcc -c  -TP -nologo  conftest.C 1>&5
c:/Program Files/Microsoft Visual Studio 8/VC/ce/bin/x86_arm/cl.exe /I"c:/Program Files/Microsoft Visual Studio 8/SmartDevices/SDK/PocketPC2003/Include" /I"x:/builds/minimo/mozilla/build/wince/shunt/include/" /FI"mozce_shunt.h" /DMOZCE_STATIC_BUILD /D_WIN32_WCE=420 /DUNDER_CE=420 /DWIN32_PLATFORM_PSPC=400 /DARM /DWINCE /D_ARM_ /Zc:wchar_t- /GS- /GR- -c -TP -nologo conftest.C 

conftest.C

configure:5443: arm-wince-gcc -c  -TP -nologo  conftest.C 1>&5
c:/Program Files/Microsoft Visual Studio 8/VC/ce/bin/x86_arm/cl.exe /I"c:/Program Files/Microsoft Visual Studio 8/SmartDevices/SDK/PocketPC2003/Include" /I"x:/builds/minimo/mozilla/build/wince/shunt/include/" /FI"mozce_shunt.h" /DMOZCE_STATIC_BUILD /D_WIN32_WCE=420 /DUNDER_CE=420 /DWIN32_PLATFORM_PSPC=400 /DARM /DWINCE /D_ARM_ /Zc:wchar_t- /GS- /GR- -c -TP -nologo conftest.C 

conftest.C

configure(5443) : error C2118: negative subscript

configure:5478: checking for 64-bit OS
configure:5487: arm-wince-gcc -c  -TC -nologo  conftest.c 1>&5
c:/Program Files/Microsoft Visual Studio 8/VC/ce/bin/x86_arm/cl.exe /I"c:/Program Files/Microsoft Visual Studio 8/SmartDevices/SDK/PocketPC2003/Include" /I"x:/builds/minimo/mozilla/build/wince/shunt/include/" /FI"mozce_shunt.h" /DMOZCE_STATIC_BUILD /D_WIN32_WCE=420 /DUNDER_CE=420 /DWIN32_PLATFORM_PSPC=400 /DARM /DWINCE /D_ARM_ /Zc:wchar_t- /GS- /GR- -c -TC -nologo conftest.c 

conftest.c

configure(5487) : error C2118: negative subscript



arm-wince-gcc is a wrapper around the arm version of cl.

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

That output looks fine.  Could you try debugging with echo statements in configure.in (or configure)?  That output shows the compilation failing on the second and fourth tests, as it should (and then again for the 64-bit check, as it should).

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

That said, I don't think the log in comment 17 matches the output in comment 15, since the log in comment 17 continues after the output in comment 15 fails with an error.