evgeny777 added inline comments.
================ Comment at: clang/test/CodeGenCXX/lto-visibility-inference.cpp:73 c1->f(); - // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C2" + // ITANIUM: type.test{{.*}}!"_ZTS2C2" // MS: type.test{{.*}}!"?AUC2@@" ---------------- tejohnson wrote: > evgeny777 wrote: > > What caused this and other changes in this file? > Because we now will insert type tests for non-hidden vtables. This is enabled > by the changes to LTO to interpret these based on the vcall_visibility > metadata. The results of this test case ``` %clang_cc1 -flto -triple x86_64-pc-windows-msvc -std=c++11 -fms-extensions -fwhole-program-vtables -flto-visibility-public-std -emit-llvm -o - %s | FileCheck --check-prefix=MS --check-prefix=MS-NOSTD %s ``` look not correct to me. I think you shouldn't generate type tests for standard library classes with `-flto-visibility-public-std`. Currently if this flag is given, clang doesn't do this either even with `-fvisibility=hidden` ================ Comment at: clang/test/CodeGenCXX/thinlto-distributed-type-metadata.cpp:7 +// as expected. +// RUN: %clang_cc1 -flto=thin -flto-unit -triple x86_64-unknown-linux -fvisibility hidden -fwhole-program-vtables -emit-llvm-bc -o %t.o %s +// RUN: llvm-dis %t.o -o - | FileCheck --check-prefix=TT %s ---------------- I think, we no longer need `-fvisibility hidden` here, do we? ================ Comment at: llvm/lib/LTO/ThinLTOCodeGenerator.cpp:976 + // via the internal option. Must be done before WPD below. + updateVCallVisibilityInIndex(*Index); + ---------------- evgeny777 wrote: > ditto updateVCallVisibilityInIndex(*Index, /* WholeProgramVisibilityEnabledInLTO */ false) ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71913/new/ https://reviews.llvm.org/D71913 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits