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

Add LTO build option #505

Merged
merged 3 commits into from
Jul 12, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ BUILD_LIBC_TOP_HALF ?= yes
BUILD_LIBSETJMP ?= yes
# The directory where we will store intermediate artifacts.
OBJDIR ?= build/$(TARGET_TRIPLE)

# LTO; no, full, or thin
# Note: thin LTO here is just for experimentation. It has known issues:
# - https://github.com/llvm/llvm-project/issues/91700
# - https://github.com/llvm/llvm-project/issues/91711
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to these issue I think you will find that any symbols that implement libcalls, or required by the libcall implemenation must currently be excluded from LTO. See https://github.com/emscripten-core/emscripten/blob/3de1b9091adbadc46706959bd97efa033214bc2c/tools/system_libs.py#L1007-L1020

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it still necessary after https://reviews.llvm.org/D71738? do you have a test case?

Copy link
Contributor Author

@yamt yamt Jun 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have excluded __cxa_atexit since then.
other things listed in your emscripten link seem like compiler-rt, which i don't have a plan to build with LTO for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think one issue is that compiler-rt can have transitive dependencies into libc. I imagine you will run into similar issues here once you start testing more extensively with LTO, but perhaps I'm wrong, or perhaps things have changed in some way.

Anyway, I guess its fine to land this and then fix issues as they come up in the field (which is basically what we did in emscripten).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i glanced at compiler-rt.
i couldn't find problematic transitive dependencies.
obvious dependencies like memset etc are covered by https://reviews.llvm.org/D71738.

LTO ?= no
ifneq ($(LTO),no)
CLANG_VERSION ?= $(shell ${CC} -dumpversion)
override OBJDIR := $(OBJDIR)/llvm-lto/$(CLANG_VERSION)
endif
# The directory where we store files and tools for generating WASIp2 bindings
BINDING_WORK_DIR ?= build/bindings
# URL from which to retrieve the WIT files used to generate the WASIp2 bindings
Expand Down Expand Up @@ -396,6 +406,18 @@ ASMFLAGS += -matomics
CFLAGS += -I$(LIBC_BOTTOM_HALF_CLOUDLIBC_SRC)
endif

ifneq ($(LTO),no)
ifeq ($(LTO),full)
CFLAGS += -flto=full
else
ifeq ($(LTO),thin)
CFLAGS += -flto=thin
else
$(error unknown LTO value: $(LTO))
endif
endif
endif

ifeq ($(WASI_SNAPSHOT), p2)
CFLAGS += -D__wasilibc_use_wasip2
endif
Expand Down Expand Up @@ -456,6 +478,9 @@ LIBC_BOTTOM_HALF_CRT_OBJS = $(call objs,$(LIBC_BOTTOM_HALF_CRT_SOURCES))
# These variables describe the locations of various files and
# directories in the generated sysroot tree.
SYSROOT_LIB := $(SYSROOT)/lib/$(TARGET_TRIPLE)
ifneq ($(LTO),no)
override SYSROOT_LIB := $(SYSROOT_LIB)/llvm-lto/$(CLANG_VERSION)
endif
SYSROOT_INC = $(SYSROOT)/include/$(TARGET_TRIPLE)
SYSROOT_SHARE = $(SYSROOT)/share/$(TARGET_TRIPLE)

Expand Down Expand Up @@ -794,12 +819,14 @@ finish: startup_files libc dummy_libs
# The build succeeded! The generated sysroot is in $(SYSROOT).
#

ifeq ($(LTO),no)
# The check for defined and undefined symbols expects there to be a heap
# alloctor (providing malloc, calloc, free, etc). Skip this step if the build
# is done without a malloc implementation.
ifneq ($(MALLOC_IMPL),none)
finish: check-symbols
endif
endif

DEFINED_SYMBOLS = $(SYSROOT_SHARE)/defined-symbols.txt
UNDEFINED_SYMBOLS = $(SYSROOT_SHARE)/undefined-symbols.txt
Expand Down