Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows libuv build not optimized #6315

Closed
thestinger opened this issue May 8, 2013 · 3 comments
Closed

windows libuv build not optimized #6315

thestinger opened this issue May 8, 2013 · 3 comments
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-slow Issue: Problems and improvements with respect to performance of generated code. O-windows Operating system: Windows

Comments

@thestinger
Copy link
Contributor

Just a minor issue to figure out in the future.

@emberian
Copy link
Member

emberian commented Jul 7, 2013

Still relevant

@huonw
Copy link
Member

huonw commented Aug 19, 2013

Triage visit: appears to still be an issue. The first few lines of output of make -n VERBOSE=1 rt/<platform>/stage0/libuv/libuv.a (with extra line breaks):

i686-pc-mingw32

gcc -I/d/stone/rust-vanilla/src/libuv/include -I/d/stone/rust-vanilla/src/libuv/include/uv-private 
    -g --std=gnu89 -D_WIN32_WINNT=0x0600 -I/d/stone/rust-vanilla/src/libuv/include 
    -I/d/stone/rust-vanilla/src/libuv/include/uv-private  -c 
    -o /d/stone/rust-vanilla/src/libuv/src/win/async.o
    /d/stone/rust-vanilla/src/libuv/src/win/async.c
gcc -I/d/stone/rust-vanilla/src/libuv/include -I/d/stone/rust-vanilla/src/libuv/include/uv-private 
    -g --std=gnu89 -D_WIN32_WINNT=0x0600 -I/d/stone/rust-vanilla/src/libuv/include 
    -I/d/stone/rust-vanilla/src/libuv/include/uv-private  -c
    -o /d/stone/rust-vanilla/src/libuv/src/win/core.o
    /d/stone/rust-vanilla/src/libuv/src/win/core.c
gcc -I/d/stone/rust-vanilla/src/libuv/include -I/d/stone/rust-vanilla/src/libuv/include/uv-private 
    -g --std=gnu89 -D_WIN32_WINNT=0x0600 -I/d/stone/rust-vanilla/src/libuv/include 
    -I/d/stone/rust-vanilla/src/libuv/include/uv-private  -c 
    -o /d/stone/rust-vanilla/src/libuv/src/win/dl.o
    /d/stone/rust-vanilla/src/libuv/src/win/dl.c

x86_64-unknown-linux-gnu

gcc --std=c89 -pedantic -Wall -Wextra -Wno-unused-parameter -D_GNU_SOURCE 
    -I/home/huon/rust/src/libuv/include -I/home/huon/rust/src/libuv/include/uv-private 
    -I/home/huon/rust/src/libuv/src -D_LARGEFILE_SOURCE 
    -D_FILE_OFFSET_BITS=64 -DRUST_DEBUG -fno-omit-frame-pointer 
    -DUSE_UTF8 -O2 -m64 -fPIC  -c /home/huon/rust/src/libuv/src/unix/async.c 
    -o src/unix/async.o
gcc --std=c89 -pedantic -Wall -Wextra -Wno-unused-parameter -D_GNU_SOURCE
    -I/home/huon/rust/src/libuv/include -I/home/huon/rust/src/libuv/include/uv-private
    -I/home/huon/rust/src/libuv/src -D_LARGEFILE_SOURCE 
    -D_FILE_OFFSET_BITS=64 -DRUST_DEBUG -fno-omit-frame-pointer
    -DUSE_UTF8 -O2 -m64 -fPIC  -c /home/huon/rust/src/libuv/src/unix/core.c
    -o src/unix/core.o
gcc --std=c89 -pedantic -Wall -Wextra -Wno-unused-parameter -D_GNU_SOURCE
    -I/home/huon/rust/src/libuv/include -I/home/huon/rust/src/libuv/include/uv-private
    -I/home/huon/rust/src/libuv/src -D_LARGEFILE_SOURCE
    -D_FILE_OFFSET_BITS=64 -DRUST_DEBUG -fno-omit-frame-pointer
    -DUSE_UTF8 -O2 -m64 -fPIC  -c /home/huon/rust/src/libuv/src/unix/dl.c
    -o src/unix/dl.o

The flags are extremely different for the former: it appears that none of our CFLAGS get passed to libuv.

Also, as @klutzy points out, there's a FIXME in rt.mk:

# FIXME: For some reason libuv's makefiles can't figure out the
# correct definition of CC on the mingw I'm using, so we are
# explicitly using gcc. Also, we have to list environment variables
# first on windows... mysterious

@klutzy
Copy link
Contributor

klutzy commented Aug 19, 2013

The issue occurs on windows only because CFLAGS are passed on other platforms:
#6176 added CFLAGS for all platforms but due to regression issues (#6253 #6279) windows stuff was removed by #6295.
$(CC) is also not explicitly passed on windows. It's the answer for FIXME.

Currently we pass CC and CFLAGS to libuv/Makefile explicitly (from mk/rt.mk):

    $$(Q)$$(MAKE) -C $$(S)src/libuv/ \
        CFLAGS="$$(CFG_GCCISH_CFLAGS) $$(LIBUV_FLAGS_$$(HOST_$(1))) $$(SNAP_DEFINES)" \
        LDFLAGS="$$(CFG_GCCISH_LINK_FLAGS) $$(LIBUV_FLAGS_$$(HOST_$(1)))" \
        CC="$$(CC_$(1))" \
        CXX="$$(CXX_$(1))" \
        AR="$$(AR_$(1))" \
        builddir_name="$$(CFG_BUILD_DIR)/rt/$(1)/stage$(2)/libuv" \
        V=$$(VERBOSE)

However, this prevents any redefinition of CFLAGS in libuv's Makefile: any CFLAGS=... or CFLAGS+=... are ignored if CFLAGS=... is passed by argument.

Yes, it can be problematic for any platform, but regression was only fatal on windows. It's because libuv has two sub makefiles named config-mingw.mk and config-unix.mk and they differ greatly.
config-mingw.mk has

CFLAGS=$(CPPFLAGS) -g --std=gnu89 -D_WIN32_WINNT=0x0600

where CPPFLAGS += -I$(SRCDIR)/include -I$(SRCDIR)/include/uv-private (libuv/build.mk). Since the line was ignored, include paths became wrong. (Also note that CFLAGS is just defined, not appended)

By contrast, config-unix.mk only contains CFLAGS += -g. You can see @huonw's result so that there's no "-g" on linux.
There may be more lurking issues due to this.

The simplest solution is adding override keyword at config-*.mk and replacing "=" with "+=" at config-mingw.mk e.g.:

override CFLAGS+=$(CPPFLAGS) -g --std=gnu89 -D_WIN32_WINNT=0x0600

I guess there exists a better solution but don't know :)

flip1995 pushed a commit to flip1995/rust that referenced this issue Nov 20, 2020
…ge_contains_using_float, r=llogiq

Fix suggestion in `manual_range_contains` when using float

Fix rust-lang#6315

changelog: Fix suggestion in `manual_range_contains` when using float
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-slow Issue: Problems and improvements with respect to performance of generated code. O-windows Operating system: Windows
Projects
None yet
Development

No branches or pull requests

4 participants