[PATCH] D40567: Always show template parameters in IR type names
sepavloff abandoned this revision. sepavloff added a comment. The feedback here, in https://reviews.llvm.org/D40508 and in the mail list (http://lists.llvm.org/pipermail/llvm-dev/2017-December/119585.html) demonstrates that this is a wrong direction. Part of this patch is used in https://reviews.llvm.org/D43805, which implements in some sense an opposite approach, - using nameless llvm types. Repository: rC Clang https://reviews.llvm.org/D40567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40567: Always show template parameters in IR type names
sepavloff created this revision. Herald added subscribers: JDevlieghere, eraman. If a module contains opaque type and another one contains definition of equivalent type in sense of C++, `llvm-link` merges these types. In this procedure it can rely only on type name. An issue arises if the type is a class template specialization. See the example. File 1 template struct ABC; ABC *var_1; File 2 template struct ABC { T *f; }; extern ABC var_1; ABC var_2; llvm-link produces module: %struct.ABC = type { i32** } @var_1 = global %struct.ABC* null, align 8 @var_2 = global %struct.ABC zeroinitializer, align 8 Incomplete type `ABC` from the first module becomes `ABC`. It happens because structure types obtained from template specialization share the same type name and differ from each other only by version suffixes, in the example the types are named as `ABC` and `ABC.0`. `llvm-link` cannot correctly merge these types. This change enables template arguments in class template specializations. Clang already prints them in class context, now they will appear in the class name itself. https://reviews.llvm.org/D40567 Files: include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def include/clang/Frontend/CodeGenOptions.h lib/CodeGen/CodeGenAction.cpp lib/CodeGen/CodeGenTypes.cpp lib/Frontend/CompilerInvocation.cpp test/CodeGenCXX/apple-kext-indirect-call.cpp test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp test/CodeGenCXX/captured-statements.cpp test/CodeGenCXX/const-init-cxx11.cpp test/CodeGenCXX/constructor-init.cpp test/CodeGenCXX/ctor-dtor-alias.cpp test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp test/CodeGenCXX/debug-info-template.cpp test/CodeGenCXX/dllexport-pr26549.cpp test/CodeGenCXX/dllexport.cpp test/CodeGenCXX/dllimport.cpp test/CodeGenCXX/float128-declarations.cpp test/CodeGenCXX/float16-declarations.cpp test/CodeGenCXX/mangle-abi-tag.cpp test/CodeGenCXX/mangle-ms-templates-memptrs-2.cpp test/CodeGenCXX/microsoft-abi-extern-template.cpp test/CodeGenCXX/microsoft-abi-vftables.cpp test/CodeGenCXX/ms-property.cpp test/CodeGenCXX/noinline-template.cpp test/CodeGenCXX/pr29160.cpp test/CodeGenCXX/static-init-3.cpp test/CodeGenCXX/template-anonymous-types.cpp test/CodeGenCXX/template-instantiation.cpp test/CodeGenCXX/template-linkage.cpp test/CodeGenCXX/value-init.cpp test/CodeGenCXX/virtual-base-destructor-call.cpp test/CodeGenCoroutines/coro-await.cpp test/CodeGenObjCXX/arc-cxx11-init-list.mm test/CodeGenObjCXX/property-lvalue-capture.mm test/OpenMP/declare_reduction_codegen.cpp test/OpenMP/target_codegen.cpp test/OpenMP/target_parallel_codegen.cpp test/OpenMP/target_simd_codegen.cpp test/OpenMP/target_teams_codegen.cpp Index: test/OpenMP/target_teams_codegen.cpp === --- test/OpenMP/target_teams_codegen.cpp +++ test/OpenMP/target_teams_codegen.cpp @@ -303,7 +303,7 @@ // CHECK-NEXT: br i1 [[ERROR]], label %[[FAIL:[^,]+]], label %[[END:[^,]+]] // CHECK: [[FAIL]] - // CHECK: call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}) + // CHECK: call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, %"struct.TT"{{[^,]+}}) // CHECK-NEXT: br label %[[END]] // CHECK: [[END]] #pragma omp target teams if(target: n>20) Index: test/OpenMP/target_simd_codegen.cpp === --- test/OpenMP/target_simd_codegen.cpp +++ test/OpenMP/target_simd_codegen.cpp @@ -292,7 +292,7 @@ // CHECK-NEXT: br i1 [[ERROR]], label %[[FAIL:[^,]+]], label %[[END:[^,]+]] // CHECK: [[FAIL]] - // CHECK: call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}) + // CHECK: call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, %"struct.TT"*{{[^,]+}}) // CHECK-NEXT: br label %[[END]] // CHECK: [[END]] #pragma omp target simd if(target: n>20) Index: test/OpenMP/target_parallel_codegen.cpp === --- test/OpenMP/target_parallel_codegen.cpp +++ test/OpenMP/target_parallel_codegen.cpp @@ -279,7 +279,7 @@ // CHECK-NEXT: br i1 [[ERROR]], label %[[FAIL:[^,]+]], label %[[END:[^,]+]] // CHECK: [[FAIL]] - // CHECK: call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}) + // CHECK: call void [[HVT4:@.+]]({{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, %"struct.TT{{[^,]+}}) // CHECK-NEXT: br label %[[END]] // CHECK: [[END]] #pragma omp target parallel if(target: n>20) In
[PATCH] D40567: Always show template parameters in IR type names
rsmith added a comment. What actual problem is this solving? These IR type names exist only for the convenience of humans reading the IR, and making them long (potentially extremely long) by including template arguments seems likely to hinder that more than it helps. https://reviews.llvm.org/D40567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40567: Always show template parameters in IR type names
sepavloff added a comment. In https://reviews.llvm.org/D40567#940025, @rsmith wrote: > What actual problem is this solving? These IR type names exist only for the > convenience of humans reading the IR, and making them long (potentially > extremely long) by including template arguments seems likely to hinder that > more than it helps. There is a case when IR type names are essential. If `llvm-link` sees opaque type in one TU, it tries to merge it with its definition in another TU. The type name is used to identify proper definition. All template specializations share the same name, so `llvm-link` has to choose random type, which results in incorrect IR. Such IR prevents from some whole-program analyses. This problem cannot be solved by `llvm-link`, as some information is already lost. Possible IR bloating due to long type names is indeed an issue. https://reviews.llvm.org/D40508 tries to solve it by replacing long type name or part of it by SHA1 hash. Type names become shorter and still are usable for type identification across different TUs. Template arguments are added to the type names only if compilation produces IR (including embedded IR in LTO), machine code generation is not affected. If adding template arguments in all cases is not appropriate, probably it makes sense to have an option, something like '--precise-ir', that would turn this generation on? It could be used in the cases when accurate type information in IR is required. https://reviews.llvm.org/D40567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40567: Always show template parameters in IR type names
sepavloff added a comment. Consider a bit more complicated example. File `common.h` #ifndef COMMON_H #define COMMON_H struct Alarm { }; struct Info { }; template struct Handler; #endif File `use-1.cpp` #include "common.h" Handler *info; void set(Handler *e) { info = e; } File `use-2.cpp` #include "common.h" Handler *alarm; void set(Handler *e) { alarm = e; } File `defs.h` #ifndef DEFS_H #define DEFS_H template<> struct Handler { int val; }; template<> struct Handler { char val; }; void set(Handler *); void set(Handler *); #endif File `main.cpp` #include "common.h" #include "defs.h" void init() { Handler i; set(&i); Handler a; set(&a); } Process these files with commands: clang -c -emit-llvm use-1.cpp -o use-1.cpp.bc clang -c -emit-llvm use-2.cpp -o use-2.cpp.bc clang -c -emit-llvm main.cpp -o main.cpp.bc llvm-link use-1.cpp.bc use-2.cpp.bc main.cpp.bc -S -o test.ll The resulting IR file has deficiencies: - The types of the global variables `info` and `alarm` are the same: %struct.Event = type { i32 } %struct.Event.0 = type { i8 } @info = global %struct.Event* null, align 8 @alarm = global %struct.Event* null, align 8 - The signatures of the `set` functions are the same: define void @_Z4fireP5EventI4InfoE(%struct.Event* %e) #0 { … } define void @_Z4fireP5EventI5AlarmE(%struct.Event* %e) #0 { … } - The call to `set(Handler *)` is augmented by a bitcast to 'fix' its type: define void @_Z4fireP5EventI5AlarmE(%struct.Event* %e) #0 { … call void bitcast (void (%struct.Event*)* @_Z4fireP5EventI5AlarmE to void (%struct.Event.0*)*)(%struct.Event.0* %a) … } The IR created by `llvm-link` is distorted. Although code generation (in LTO compilation) might be unaffected by this distortions, other applications of IR linking suffer from it. It does not allow to implement some checks, validation techniques and optimizations. https://reviews.llvm.org/D40567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40567: Always show template parameters in IR type names
sepavloff updated this revision to Diff 125761. sepavloff added a comment. Updated patch - old type names are generated always if --ir-type-names is not specified, - added new value of --ir-type-names, none, to suppress type names, - value of --ir-type-names is stored in module properties. Repository: rC Clang https://reviews.llvm.org/D40567 Files: include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def include/clang/Frontend/CodeGenOptions.h lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenTypes.cpp lib/Driver/ToolChains/Clang.cpp lib/Frontend/CompilerInvocation.cpp test/CodeGenCXX/template-types.cpp Index: test/CodeGenCXX/template-types.cpp === --- /dev/null +++ test/CodeGenCXX/template-types.cpp @@ -0,0 +1,118 @@ +// RUN: %clang_cc1 %s -S -emit-llvm -o - | FileCheck %s +// RUN: %clang_cc1 %s -S -emit-llvm --ir-type-names=none -o - | FileCheck %s --check-prefix=CHECK-NONE +// RUN: %clang_cc1 %s -S -emit-llvm --ir-type-names=terse -o - | FileCheck %s --check-prefix=CHECK-TERSE +// RUN: %clang_cc1 %s -S -emit-llvm --ir-type-names=full -o - | FileCheck %s --check-prefix=CHECK-FULL + +struct Empty {}; +template struct ABC { + T v; +}; +struct Opaque; +template class OpaqueClass; + +ABC var_1; +ABC > var_2; +ABC var_3; +ABC *var_4; +OpaqueClass *var_5; +OpaqueClass *var_6; +OpaqueClass *var_7; +OpaqueClass > *var_8; + + +//-- without option '--ir-type-names' + +// CHECK: %struct.ABC = type { i32 } +// CHECK: %struct.ABC.0 = type { %struct.ABC.1 } +// CHECK: %struct.ABC.1 = type { i16 } +// CHECK: %struct.ABC.2 = type { %struct.Empty } +// CHECK: %struct.Empty = type { i8 } +// CHECK: %class.OpaqueClass = type opaque +// CHECK: %class.OpaqueClass.4 = type opaque +// CHECK: %class.OpaqueClass.5 = type opaque +// CHECK: %class.OpaqueClass.6 = type opaque + +// CHECK: @var_1 = global %struct.ABC zeroinitializer +// CHECK: @var_2 = global %struct.ABC.0 zeroinitializer +// CHECK: @var_3 = global %struct.ABC.2 zeroinitializer +// CHECK: @var_4 = global %struct.ABC.3* null +// CHECK: @var_5 = global %class.OpaqueClass* null +// CHECK: @var_6 = global %class.OpaqueClass.4* null +// CHECK: @var_7 = global %class.OpaqueClass.5* null +// CHECK: @var_8 = global %class.OpaqueClass.6* null + +// CHECK-NOT: !{{[0-9]+}} = !{{{.*}} !"type_names" + + +//-- with option '--ir-type-names=none' + +// CHECK-NONE: %0 = type { i32 } +// CHECK-NONE: %1 = type { %2 } +// CHECK-NONE: %2 = type { i16 } +// CHECK-NONE: %3 = type { %4 } +// CHECK-NONE: %4 = type { i8 } +// CHECK-NONE: %5 = type opaque +// CHECK-NONE: %6 = type opaque +// CHECK-NONE: %7 = type opaque +// CHECK-NONE: %8 = type opaque +// CHECK-NONE: %9 = type opaque + +// CHECK-NONE: @var_1 = global %0 zeroinitializer +// CHECK-NONE: @var_2 = global %1 zeroinitializer +// CHECK-NONE: @var_3 = global %3 zeroinitializer +// CHECK-NONE: @var_4 = global %5* null +// CHECK-NONE: @var_5 = global %6* null +// CHECK-NONE: @var_6 = global %7* null +// CHECK-NONE: @var_7 = global %8* null +// CHECK-NONE: @var_8 = global %9* null + +// CHECK-NONE: !{{[0-9]+}} = !{{{.*}} !"type_names", i32 1} + + +//-- with option '--ir-type-names=terse' + +// CHECK-TERSE: %struct.ABC = type { i32 } +// CHECK-TERSE: %struct.ABC.0 = type { %struct.ABC.1 } +// CHECK-TERSE: %struct.ABC.1 = type { i16 } +// CHECK-TERSE: %struct.ABC.2 = type { %struct.Empty } +// CHECK-TERSE: %struct.Empty = type { i8 } +// CHECK-TERSE: %class.OpaqueClass = type opaque +// CHECK-TERSE: %class.OpaqueClass.4 = type opaque +// CHECK-TERSE: %class.OpaqueClass.5 = type opaque +// CHECK-TERSE: %class.OpaqueClass.6 = type opaque + +// CHECK-TERSE: @var_1 = global %struct.ABC zeroinitializer +// CHECK-TERSE: @var_2 = global %struct.ABC.0 zeroinitializer +// CHECK-TERSE: @var_3 = global %struct.ABC.2 zeroinitializer +// CHECK-TERSE: @var_4 = global %struct.ABC.3* null +// CHECK-TERSE: @var_5 = global %class.OpaqueClass* null +// CHECK-TERSE: @var_6 = global %class.OpaqueClass.4* null +// CHECK-TERSE: @var_7 = global %class.OpaqueClass.5* null +// CHECK-TERSE: @var_8 = global %class.OpaqueClass.6* null + +// CHECK-TERSE: !{{[0-9]+}} = !{{{.*}} !"type_names", i32 2} + + +//-- with option '--ir-type-names=full' + +// CHECK-FULL: %"struct.ABC" = type { i32 } +// CHECK-FULL: %"struct.ABC >" = type { %"struct.ABC" } +// CHECK-FULL: %"struct.ABC" = type { i16 } +// CHECK-FULL: %"struct.ABC" = type { %struct.Empty } +// CHECK-FULL: %struct.Empty = type { i8 } +// CHECK-FULL: %"struct.ABC" = type opaque +// CHECK-FULL: %"class.OpaqueClass" = type opaque +// CHECK-FULL: %"class.OpaqueClass" = type opaque +// CHECK-FULL: %"class.OpaqueClass" = type opaque +// CHECK-FULL: %"class.OpaqueClass >" = type opaque + +// CHECK-FULL: @var_1 = global %"struct.ABC" zeroinitializer +// CHECK-FULL: @var_2 = global %"struct.ABC >" zeroinitializer +// CHECK-FULL: @var_3 = global %"struct.ABC" zeroinitializer +// CHECK-FULL: @var_4 = globa
[PATCH] D40567: Always show template parameters in IR type names
rsmith added a comment. In https://reviews.llvm.org/D40567#943747, @sepavloff wrote: > Although code generation (in LTO compilation) might be unaffected by this > distortions, other applications of IR linking suffer from it. It does not > allow to implement some checks, validation techniques and optimizations. Any system trying to use IR type names to deduce information about source-level types is simply wrong. We simply don't provide the sort of guarantees you seem to be looking for here. We don't even guarantee to consistently use the same IR type for the same source type within a single translation unit. IR type names exist only for the benefit of humans reading the IR. Repository: rC Clang https://reviews.llvm.org/D40567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits