Skip to content

Conversation

@google-yfyang
Copy link
Collaborator

@google-yfyang google-yfyang commented Jun 18, 2025

Importing llvm-libc from main instead of llvm-20.

Add some basic file filtering and patch applying logic in the import script system/lib/update_llvm_libc.py.

Tested:
The same number of tests failing at HEAD when running ./test/runner llvmlibc.
#24493

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Nice work!

Lets see if all the tests pass (i.e. lets see if libc++ is effected by this change).

Also, can you remove the llvm-libc stuff from system/lib/update_libcxx.py

script_dir = os.path.abspath(os.path.dirname(__file__))
emscripten_root = os.path.dirname(os.path.dirname(script_dir))
default_llvm_dir = os.path.join(os.path.dirname(emscripten_root), 'llvm-project')
emscripten_patches = os.path.join(script_dir, "llvm-libc", "patches", "*.patch")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't tend to check in patches like this, at least not for the other libraries here. Instead we push our changes out to https://github.com/emscripten-core/llvm-project and maintain the patches in git.

I'm not totally opposed to this approach here, but it would be new thing for us.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suppose those patches are not submitted to the upstream?

If I understand correctly, you want me to submit the content of this patch to emscripten's fork of llvm?

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Also, can you update the readme file in the llvm-libc directory?

@google-yfyang
Copy link
Collaborator Author

Also, can you update the readme file in the llvm-libc directory?

Done.

Nice work!

Lets see if all the tests pass (i.e. lets see if libc++ is effected by this change).

Also, can you remove the llvm-libc stuff from system/lib/update_libcxx.py

Tests are failing due to some differences in the expected code size. Can you give me a pointer on how those are updated?

@sbc100 sbc100 changed the title import the already imported part of llvm-libc from HEAD [llvm-libc] Import the already imported part of llvm-libc from llvm main Jul 10, 2025
@google-yfyang google-yfyang merged commit 4faf26f into emscripten-core:main Jul 11, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants