[PATCH] D51905: Front-end of the implementation of the interleaving algorithm

Vlad Tsyrklevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 14 14:14:44 PDT 2018


vlad.tsyrklevich added a comment.

In https://reviews.llvm.org/D51905#1234308, @zhaomo wrote:

> In https://reviews.llvm.org/D51905#1231383, @vlad.tsyrklevich wrote:
>
> > This change causes all compiler-rt cfi tests to be UNSUPPORTED for me locally, do you have any idea why that might be? The lit changes don't make it immediately clear.
>
>
> Not sure why that happened. How did you run the compiler-rt tests? Did you use ninja check-cfi?


I ran them with llvm-lit against the tests under test/cfi. It looks like something caused the default test suite to change where all of the tests were marked unsupported (the test suite ran without lld and my default linker otherwise is bfd so LTO was unsupported.) Not a problem with the change.



================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:939
 
+/// We should only interleave vtables when the module has the hidden
+/// LTO visibility, cfi-vcall is enabled and EnableVTableInterleaving
----------------
zhaomo wrote:
> vlad.tsyrklevich wrote:
> > doxygen comments normally go in the corresponding header file.
> I just followed the style of the rest of the file. All the comments above functions use ///. Do I need to change them?
Disregard that, I hadn't realized that was the style elsewhere.


https://reviews.llvm.org/D51905





More information about the cfe-commits mailing list