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

[C++20][Modules] Missed optimization: Unnecessary code emitted for empty module initializers #67582

Closed
davidstone opened this issue Sep 27, 2023 · 2 comments
Labels
clang:modules C++20 modules and Clang Header Modules duplicate Resolved as duplicate

Comments

@davidstone
Copy link
Contributor

davidstone commented Sep 27, 2023

Given

a.cpp:

export module a;

and

b.cpp:

import a;

Then after compiling with

clang++ -std=c++20 -O3 -x c++-module -fmodule-output=a.pcm --precompile -c a.cpp
clang++ -std=c++20 -O3 -fmodule-file=a=a.pcm -S b.cpp -emit-llvm

We end up with the generated code for b as:

; ModuleID = '/home/david/test/b.cpp'
source_filename = "/home/david/test/b.cpp"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

@llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__sub_I_b.cpp, ptr null }]

declare void @_ZGIW1a() local_unnamed_addr

; Function Attrs: uwtable
define internal void @_GLOBAL__sub_I_b.cpp() #0 section ".text.startup" {
entry:
  tail call void @_ZGIW1a()
  ret void
}

attributes #0 = { uwtable "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }

!llvm.linker.options = !{}
!llvm.module.flags = !{!0, !1, !2, !3}
!llvm.ident = !{!4}

!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{i32 8, !"PIC Level", i32 2}
!2 = !{i32 7, !"PIE Level", i32 2}
!3 = !{i32 7, !"uwtable", i32 2}
!4 = !{!"clang version 18.0.0 (https://github.com/llvm/llvm-project.git cd7f1714dea16261e0362b80958296f9782d625e)"}

The module initializer for a is empty, yet we still do a tail call to it. I would expect there to be no reference to the initializer of module a anywhere in the generated code for b.

It actually seems to be significantly worse than this. For instance, in my local compiler explorer instance I have an umbrella module bounded that imports various sub-modules. I see an unnecessary initializer of the umbrella module (which internally has unnecessary initializers for all its imports), and then I see unnecessary initializers of all of those sub-modules:

import bounded;

In my compiler explorer output I see

_GLOBAL__sub_I_example.cpp:             # @_GLOBAL__sub_I_example.cpp
        push    rax
        call    initializer for module bounded@PLT
        call    initializer for module bounded.arithmetic.operators@PLT
        call    initializer for module bounded.arithmetic.operators_builtin@PLT
        call    initializer for module bounded.arithmetic.round_up_divide@PLT
        call    initializer for module bounded.abs@PLT
        call    initializer for module bounded.builtin_min_max_value@PLT
        call    initializer for module bounded.cast@PLT
        call    initializer for module bounded.check_in_range@PLT
        call    initializer for module bounded.clamp@PLT
        call    initializer for module bounded.comparison@PLT
        call    initializer for module bounded.comparison_builtin@PLT
        call    initializer for module bounded.construct@PLT
        call    initializer for module bounded.construct_at@PLT
        call    initializer for module bounded.copy@PLT
        call    initializer for module bounded.hash@PLT
        call    initializer for module bounded.integer@PLT
        call    initializer for module bounded.integer_tombstone_traits@PLT
        call    initializer for module bounded.is_bounded_integer@PLT
        call    initializer for module bounded.lazy_init@PLT
        call    initializer for module bounded.literal@PLT
        call    initializer for module bounded.log@PLT
        call    initializer for module bounded.minmax@PLT
        call    initializer for module bounded.normalize@PLT
        call    initializer for module bounded.number_of@PLT
        call    initializer for module bounded.pow@PLT
        call    initializer for module bounded.representation_digits@PLT
        call    initializer for module bounded.size_of@PLT
        call    initializer for module bounded.std_iterator@PLT
        call    initializer for module bounded.stream@PLT
        pop     rax
        jmp     initializer for module bounded.to_integer@PLT  # TAILCALL
@EugeneZelenko EugeneZelenko added clang:modules C++20 modules and Clang Header Modules and removed new issue labels Sep 28, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 28, 2023

@llvm/issue-subscribers-clang-modules

Given

a.cpp:

export module a;

and

b.cpp:

import a;

Then after compiling with

clang++ -std=c++20 -O3 -x c++-module -fmodule-output=a.pcm --precompile -c a.cpp
clang++ -std=c++20 -O3 -fmodule-file=a=a.pcm -S b.cpp -emit-llvm

We end up with the generated code for b as:

; ModuleID = '/home/david/test/b.cpp'
source_filename = "/home/david/test/b.cpp"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

@<!-- -->llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @<!-- -->_GLOBAL__sub_I_b.cpp, ptr null }]

declare void @<!-- -->_ZGIW1a() local_unnamed_addr

; Function Attrs: uwtable
define internal void @<!-- -->_GLOBAL__sub_I_b.cpp() #<!-- -->0 section ".text.startup" {
entry:
  tail call void @<!-- -->_ZGIW1a()
  ret void
}

attributes #<!-- -->0 = { uwtable "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }

!llvm.linker.options = !{}
!llvm.module.flags = !{!0, !1, !2, !3}
!llvm.ident = !{!4}

!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{i32 8, !"PIC Level", i32 2}
!2 = !{i32 7, !"PIE Level", i32 2}
!3 = !{i32 7, !"uwtable", i32 2}
!4 = !{!"clang version 18.0.0 (https://github.com/llvm/llvm-project.git cd7f1714dea16261e0362b80958296f9782d625e)"}

The module initializer for a is empty, yet we still do a tail call to it. I would expect there to be no reference to the initializer of module a anywhere in the generated code for b.

It actually seems to be significantly worse than this. For instance, in my local compiler explorer instance I have an umbrella module bounded that imports various sub-modules. I see an unnecessary initializer of the umbrella module (which internally has unnecessary initializers for all its imports), and then I see unnecessary initializers of all of those sub-modules:

import bounded;

In my compiler explorer output I see

_GLOBAL__sub_I_example.cpp:             # @<!-- -->_GLOBAL__sub_I_example.cpp
        push    rax
        call    initializer for module bounded@<!-- -->PLT
        call    initializer for module bounded.arithmetic.operators@<!-- -->PLT
        call    initializer for module bounded.arithmetic.operators_builtin@<!-- -->PLT
        call    initializer for module bounded.arithmetic.round_up_divide@<!-- -->PLT
        call    initializer for module bounded.abs@<!-- -->PLT
        call    initializer for module bounded.builtin_min_max_value@<!-- -->PLT
        call    initializer for module bounded.cast@<!-- -->PLT
        call    initializer for module bounded.check_in_range@<!-- -->PLT
        call    initializer for module bounded.clamp@<!-- -->PLT
        call    initializer for module bounded.comparison@<!-- -->PLT
        call    initializer for module bounded.comparison_builtin@<!-- -->PLT
        call    initializer for module bounded.construct@<!-- -->PLT
        call    initializer for module bounded.construct_at@<!-- -->PLT
        call    initializer for module bounded.copy@<!-- -->PLT
        call    initializer for module bounded.hash@<!-- -->PLT
        call    initializer for module bounded.integer@<!-- -->PLT
        call    initializer for module bounded.integer_tombstone_traits@<!-- -->PLT
        call    initializer for module bounded.is_bounded_integer@<!-- -->PLT
        call    initializer for module bounded.lazy_init@<!-- -->PLT
        call    initializer for module bounded.literal@<!-- -->PLT
        call    initializer for module bounded.log@<!-- -->PLT
        call    initializer for module bounded.minmax@<!-- -->PLT
        call    initializer for module bounded.normalize@<!-- -->PLT
        call    initializer for module bounded.number_of@<!-- -->PLT
        call    initializer for module bounded.pow@<!-- -->PLT
        call    initializer for module bounded.representation_digits@<!-- -->PLT
        call    initializer for module bounded.size_of@<!-- -->PLT
        call    initializer for module bounded.std_iterator@<!-- -->PLT
        call    initializer for module bounded.stream@<!-- -->PLT
        pop     rax
        jmp     initializer for module bounded.to_integer@<!-- -->PLT  # TAILCALL

@ChuanqiXu9
Copy link
Member

Thanks for the report. And this is duplicated with #56794

@ChuanqiXu9 ChuanqiXu9 added the duplicate Resolved as duplicate label Sep 28, 2023
@ChuanqiXu9 ChuanqiXu9 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 28, 2023
ChuanqiXu9 added a commit to ChuanqiXu9/llvm-project that referenced this issue Sep 28, 2023
…t init anything

Close llvm#56794

And see llvm#67582 for a detailed
backgrond for the issue.

As required by the Itanium ABI, the module units have to generate the
initialization function. However, the importers are allowed to elide the
call to the initialization function if they are sure the initialization
function doesn't do anything.

This patch implemented this semantics.
ChuanqiXu9 added a commit that referenced this issue Sep 28, 2023
… init anything (#67638)

Close #56794

And see #67582 for a detailed
backgrond for the issue.

As required by the Itanium ABI, the module units have to generate the
initialization function. However, the importers are allowed to elide the
call to the initialization function if they are sure the initialization
function doesn't do anything.

This patch implemented this semantics.
legrosbuffle pushed a commit to legrosbuffle/llvm-project that referenced this issue Sep 29, 2023
… init anything (llvm#67638)

Close llvm#56794

And see llvm#67582 for a detailed
backgrond for the issue.

As required by the Itanium ABI, the module units have to generate the
initialization function. However, the importers are allowed to elide the
call to the initialization function if they are sure the initialization
function doesn't do anything.

This patch implemented this semantics.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:modules C++20 modules and Clang Header Modules duplicate Resolved as duplicate
Projects
None yet
Development

No branches or pull requests

4 participants