[PATCH] D40567: Always show template parameters in IR type names

2018-02-26 Thread Serge Pavlov via Phabricator via cfe-commits
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

2017-12-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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


[PATCH] D40567: Always show template parameters in IR type names

2017-12-06 Thread Serge Pavlov via Phabricator via cfe-commits
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" 

[PATCH] D40567: Always show template parameters in IR type names

2017-12-04 Thread Serge Pavlov via Phabricator via cfe-commits
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();
Handler a;
set();
  }

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

2017-11-29 Thread Serge Pavlov via Phabricator via cfe-commits
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

2017-11-29 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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

2017-11-28 Thread Serge Pavlov via Phabricator via cfe-commits
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