daltenty added inline comments.
================
Comment at: llvm/include/llvm/Target/TargetMachine.h:265
+ /// corresponding to -mno-xcoff-visibility.
+ bool getNoXCOFFVisibility() const { return Options.NoXCOFFVisibility; }
+
----------------
DiggerLin wrote:
> jasonliu wrote:
> > DiggerLin wrote:
> > > daltenty wrote:
> > > > This seems like it needs the corresponding comand-line option for llc
> > > > added and an llc test.
> > > I think it will be in another separate patch.
> > I would actually prefer to have that in the same patch, as that would give
> > us a full picture. It's not a huge patch even if we combine them.
> yes, it is not huge patch, one patch for the clang with option
> -mno-xcoff-visibility, another patch for llc option -no-xcoff-visibility , I
> think it is different functionality. and I have post the
> https://reviews.llvm.org/D88234 "add new option -no-xcoff-visibility for llc"
But the problem is this patch actually introduces the backend functionality
with no test in the LLVM component itself, because the test here is Clang only.
Either the patches should be merged so both components get tests in one patch
or both refactored so we have one llc/LLVM patch and one clang patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87451/new/
https://reviews.llvm.org/D87451
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
