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

Link-time-optimization with clang is broken #96761

Closed
matthiasgoergens opened this issue Sep 12, 2022 · 15 comments
Closed

Link-time-optimization with clang is broken #96761

matthiasgoergens opened this issue Sep 12, 2022 · 15 comments
Assignees
Labels
3.11 only security fixes 3.12 bugs and security fixes type-bug An unexpected behavior, bug, or error

Comments

@matthiasgoergens
Copy link
Contributor

I tried to enable link time optimizations for clang.

export CC="clang"
export LD="clang"

nice "${here}/configure" \
    --enable-optimizations \
    --with-lto=thin

nice make -j8

Eventually building reaches this step:

clang -fprofile-instr-generate -o _bootstrap_python Modules/getbuildinfo.o Parser/token.o Parser/pegen.o Parser/pegen_errors.o Parser/action_helpers.o Parser/parser.o Parser/string_parser.o Parser/peg_api.o Parser/myreadline.o Parser/tokenizer.o Objects/abstract.o Objects/boolobject.o Objects/bytes_methods.o Objects/bytearrayobject.o Objects/bytesobject.o Objects/call.o Objects/capsule.o Objects/cellobject.o Objects/classobject.o Objects/codeobject.o Objects/complexobject.o Objects/descrobject.o Objects/enumobject.o Objects/exceptions.o Objects/genericaliasobject.o Objects/genobject.o Objects/fileobject.o Objects/floatobject.o Objects/frameobject.o Objects/funcobject.o Objects/interpreteridobject.o Objects/iterobject.o Objects/listobject.o Objects/longobject.o Objects/dictobject.o Objects/odictobject.o Objects/memoryobject.o Objects/methodobject.o Objects/moduleobject.o Objects/namespaceobject.o Objects/object.o Objects/obmalloc.o Objects/picklebufobject.o Objects/rangeobject.o Objects/setobject.o Objects/sliceobject.o Objects/structseq.o Objects/tupleobject.o Objects/typeobject.o Objects/unicodeobject.o Objects/unicodectype.o Objects/unionobject.o Objects/weakrefobject.o Objects/perf_trampoline.o Objects/asm_trampoline.o Python/_warnings.o Python/Python-ast.o Python/Python-tokenize.o Python/asdl.o Python/ast.o Python/ast_opt.o Python/ast_unparse.o Python/bltinmodule.o Python/ceval.o Python/codecs.o Python/compile.o Python/context.o Python/dynamic_annotations.o Python/errors.o Python/frame.o Python/frozenmain.o Python/future.o Python/getargs.o Python/getcompiler.o Python/getcopyright.o Python/getplatform.o Python/getversion.o Python/ceval_gil.o Python/hamt.o Python/hashtable.o Python/import.o Python/importdl.o Python/initconfig.o Python/marshal.o Python/modsupport.o Python/mysnprintf.o Python/mystrtoul.o Python/pathconfig.o Python/preconfig.o Python/pyarena.o Python/pyctype.o Python/pyfpe.o Python/pyhash.o Python/pylifecycle.o Python/pymath.o Python/pystate.o Python/pythonrun.o Python/pytime.o Python/bootstrap_hash.o Python/specialize.o Python/structmember.o Python/symtable.o Python/sysmodule.o Python/thread.o Python/traceback.o Python/getopt.o Python/pystrcmp.o Python/pystrtod.o Python/pystrhex.o Python/dtoa.o Python/formatter_unicode.o Python/fileutils.o Python/suggestions.o Python/dynload_shlib.o Modules/config.o Modules/main.o Modules/gcmodule.o Modules/atexitmodule.o Modules/faulthandler.o Modules/posixmodule.o Modules/signalmodule.o Modules/_tracemalloc.o Modules/_codecsmodule.o Modules/_collectionsmodule.o Modules/errnomodule.o Modules/_io/_iomodule.o Modules/_io/iobase.o Modules/_io/fileio.o Modules/_io/bytesio.o Modules/_io/bufferedio.o Modules/_io/textio.o Modules/_io/stringio.o Modules/itertoolsmodule.o Modules/_sre/sre.o Modules/_threadmodule.o Modules/timemodule.o Modules/_weakref.o Modules/_abc.o Modules/_functoolsmodule.o Modules/_localemodule.o Modules/_operator.o Modules/_stat.o Modules/symtablemodule.o Modules/pwdmodule.o Programs/_bootstrap_python.o Modules/getpath.o -ldl -lm

And fails:

Modules/getbuildinfo.o: file not recognized: file format not recognized
clang-14: error: linker command failed with exit code 1 (use -v to see invocation)

Your environment

I'm on Archlinux x86-64.

$ clang --version
clang version 14.0.6
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

I tested against main.

@matthiasgoergens matthiasgoergens added the type-bug An unexpected behavior, bug, or error label Sep 12, 2022
@corona10
Copy link
Member

-no-lto flag was first introduced at #29859 by @tiran
Would you like to take a look at this issue please?

@matthiasgoergens
Copy link
Contributor Author

@corona10 Thanks! That seems to be mostly about speeding up the GCC build. LLVM is already pretty fast with thin-lto.

@corona10
Copy link
Member

I will try to reproduce the issue in my Linux environment with clang14 before reviewing the PR.
cc @tiran

@corona10
Copy link
Member

Okay, I am able to reproduce the issue even with the lld.

@matthiasgoergens
Copy link
Contributor Author

Thanks for confirming that it's not just some idiosyncratic weirdness of my environment.

@corona10 corona10 added 3.11 only security fixes 3.12 bugs and security fixes labels Sep 18, 2022
@corona10
Copy link
Member

corona10 commented Sep 18, 2022

For @matthiasgoergens, @tiran @vstinner cc @pablogsal as release manager.

My conclusion

  • At this moment Modules/*.o are actually LLVM bitcode not object files.
  • so the LTO processor needs to transform the LLVM bitcode into the object file for the traditional linker but looks like without -flto it doesn't transform to object files for the traditional linker.
  • This issue not only happens for ThinLTO but also in full LTO because both processes have the phase for transforming the LLVM bitcode into the object file.
  • see http://blog.llvm.org/2016/06/thinlto-scalable-and-incremental-lto.html
  • IMHO we can add the transformation phase for -fno-lto but it could be complicated, I prefer using gh-96761: Fix lto-build for clang #96762 as a workaround approach and we need to backport to Python 3.11 too.
  • But I am still curious about the fact why apple clang does not raise this issue.

Reference

file format

corona10@python-dev:~/cpython$ file  Modules/getbuildinfo.o
Modules/getbuildinfo.o: LLVM IR bitcode

clang full LTO process

image

clang ThinLTO process

image

@pablogsal
Copy link
Member

Why is this only affecting 3.11 and not previous version?

@corona10
Copy link
Member

corona10 commented Sep 18, 2022

Why is this only affecting 3.11 and not previous version?

because #29859 was first introduced in Python 3.11 for _bootstrap_python.

@corona10
Copy link
Member

corona10 commented Sep 18, 2022

When building the final binary for CPython, we actually pass the -flto flags that the process will transform the LLVM bitcode into object files, so this issue only affects while building the _bootstrap_python.
I didn't check if the issue will affect to old versions (< 3.10)

@corona10
Copy link
Member

corona10 commented Sep 18, 2022

Manual transformation example:

corona10@python-dev:~/cpython$ llc-14 -filetype=obj Modules/getbuildinfo.o -o Modules/getbuildinfo.o
corona10@python-dev:~/cpython$ file Modules/getbuildinfo.o
Modules/getbuildinfo.o: ELF 64-bit LSB relocatable, x86-64, version 1 (GNU/Linux), with debug_info, not stripped

@pablogsal
Copy link
Member

Thanks for the quick answer and great analysis @corona10. I think the fix for this should go to 3.11.1 as this likely involve configure changes unless someone fundamentally disagrees.

@matthiasgoergens
Copy link
Contributor Author

As far as I can tell, building _bootstrap_python with different flags than the full Python binary was only a hack to speed up the build, wasn't it?

Thin LTO is pretty fast, and compatible with the other LTO settings (apart perhaps from no-LTO), so we can build the bootstrap with that one, if any LTO is requested for the main build?

@corona10
Copy link
Member

corona10 commented Sep 19, 2022

Thin LTO is pretty fast, and compatible with the other LTO settings (apart perhaps from no-LTO), so we can build the bootstrap with that one, if any LTO is requested for the main build?

Please follow my last review: #96762 (comment)
Since we should support all C11 compilers including old clangs which doesn't support ThinLTO, the configuration should use ThinLTO only if the compiler supports it. We should not break the build for old compilers too.
see: https://clang.llvm.org/c_status.html and https://peps.python.org/pep-0007/#c-dialect

@matthiasgoergens
Copy link
Contributor Author

@corona10 Yes, definitely. I need to add checks.

corona10 added a commit to corona10/cpython that referenced this issue Sep 20, 2022
…p_python

Co-authored-by: Matthias Goergens <matthias.goergens@gmail.com>
corona10 added a commit that referenced this issue Sep 23, 2022
…on (gh-96945)

Co-authored-by: Matthias Goergens <matthias.goergens@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 23, 2022
…p_python (pythongh-96945)

Co-authored-by: Matthias Goergens <matthias.goergens@gmail.com>
(cherry picked from commit 83d84e6)

Co-authored-by: Dong-hee Na <donghee.na@python.org>
miss-islington added a commit that referenced this issue Sep 23, 2022
…on (gh-96945)

Co-authored-by: Matthias Goergens <matthias.goergens@gmail.com>
(cherry picked from commit 83d84e6)

Co-authored-by: Dong-hee Na <donghee.na@python.org>
@corona10
Copy link
Member

All patches are merged.
Feel free to reopen if we need other solutions.
cc @tiran @pablogsal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 bugs and security fixes type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants