Last Comment Bug 372878 - reduce the number of AC_TRY_RUN tests in configure.in
: reduce the number of AC_TRY_RUN tests in configure.in
Status: RESOLVED FIXED
[patch]
: fixed1.8.1.4
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
:
Mentors:
Depends on: 423913
Blocks:
  Show dependency treegraph
 
Reported: 2007-03-06 13:43 PST by David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
Modified: 2008-06-03 17:38 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
add static assertion macros and convert just the sizeof tests (5.51 KB, patch)
2007-03-06 14:41 PST, David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
no flags Details | Diff | Splinter Review
add static assertion macros and convert just the sizeof tests (5.87 KB, patch)
2007-03-06 15:00 PST, David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
benjamin: review+
dveditz: approval1.8.1.4+
Details | Diff | Splinter Review
convert system library version tests (3.18 KB, patch)
2007-03-08 16:28 PST, David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch)
benjamin: review+
Details | Diff | Splinter Review

Description David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2007-03-06 13:43:07 PST
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+2 (mostly busy through August 4; review requests must explain patch) 2007-03-06 13:49:04 PST
(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+2 (mostly busy through August 4; review requests must explain patch) 2007-03-06 14:41:31 PST
Created attachment 257564 [details] [diff] [review]
add static assertion macros and convert just the sizeof tests
Comment 3 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2007-03-06 15:00:28 PST
Created attachment 257567 [details] [diff] [review]
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+2 (mostly busy through August 4; review requests must explain patch) 2007-03-08 10:19:09 PST
attachment 257567 [details] [diff] [review] checked in to trunk.
Comment 5 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2007-03-08 10:55:08 PST
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 AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2007-03-08 12:24:02 PST
I think we should let this bake a couple weeks before landing on the branch.
Comment 7 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2007-03-08 16:28:07 PST
Created attachment 257891 [details] [diff] [review]
convert system library version tests
Comment 8 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2007-03-09 15:39:43 PST
And along with that I should bump the libpng version to what we actually use; bug 334110 forgot to do that.
Comment 9 Ted Mielczarek [:ted.mielczarek] 2007-03-14 12:26:03 PDT
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+2 (mostly busy through August 4; review requests must explain patch) 2007-03-22 23:34:19 PDT
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] 2007-03-29 11:43:14 PDT
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+2 (mostly busy through August 4; review requests must explain patch) 2007-03-29 12:20:29 PDT
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+2 (mostly busy through August 4; review requests must explain patch) 2007-03-29 13:45:26 PDT
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+2 (mostly busy through August 4; review requests must explain patch) 2007-03-29 13:50:48 PDT
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) 2007-05-14 23:53:08 PDT
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+2 (mostly busy through August 4; review requests must explain patch) 2007-05-15 00:40:30 PDT
The errors in config.log ?
Comment 17 Doug Turner (:dougt) 2007-05-15 09:49:13 PDT
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+2 (mostly busy through August 4; review requests must explain patch) 2007-05-15 11:24:53 PDT
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+2 (mostly busy through August 4; review requests must explain patch) 2007-05-15 11:27:12 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.