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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to