[PATCH] D42611: [ThinLTO/CFI] Include TYPE_ID summaries into GLOBALVAL_SUMMARY_BLOCK

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: clang/test/CodeGen/thinlto-distributed-cfi-devirt.cpp:7
+
+// RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu -emit-llvm-bc \
+// RUN:   -flto=thin -flto-unit -fvisibility hidden \
----------------
I wonder whether you can simplify this test by making this a `.ll` file and using `opt -thinlto-bc` to create the `.o` file.


================
Comment at: clang/test/CodeGen/thinlto-distributed-cfi-devirt.cpp:72
+// Ensure that linking works.
+// RUN: %clangxx %t.native.o %t2.index.0 -o %t
+
----------------
I think this will only work on Linux. Same goes for your other tests.


================
Comment at: clang/test/CodeGen/thinlto-distributed-cfi-devirt.cpp:74
+
+// Ensure that traps are conditional.
+// RUN: llvm-objdump -d %t | FileCheck --check-prefixes=CHECK-DIS %s
----------------
It may be simpler to test the effect of lowertypetests by passing `-emit-llvm` to clang and examining the IR.


================
Comment at: clang/test/CodeGen/thinlto-distributed-cfi.cpp:80
+
+struct A {
+  virtual int f() = 0;
----------------
Since this test and `thinlto-distributed-cfi-devirt.cpp` use the same inputs, you can merge them into one file.


https://reviews.llvm.org/D42611