jasonliu added inline comments.

================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:520
   Options.DataSections = CodeGenOpts.DataSections;
-  Options.IgnoreXCOFFVisibility = CodeGenOpts.IgnoreXCOFFVisibility;
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
----------------
DiggerLin wrote:
> jasonliu wrote:
> > DiggerLin wrote:
> > > jasonliu wrote:
> > > > Instead of just removing this line, should this get replaced with the 
> > > > new LangOpts option?
> > > I do not think we need a CodeGenOp of ignore-xcoff-visibility in clang, 
> > > we only need the LangOpt of the ignore-xcoff-visilbity to control whether 
> > > we will  generate the visibility in the IR,  when the LangOpt of 
> > > ignore-xcoff-visibility do not generate the visibility attribute of GV in 
> > > the IR. it do not need CodeGenOp of ignore-xcoff-visibility any more for 
> > > the clang .
> > > 
> > > we have still CodeGen ignore-xcoff-visibility op in  llc.
> > We removed the visibility from IR level with this patch. But there is also 
> > visibility settings coming from CodeGen part of clang, which needs to get 
> > ignore when we are doing the code gen in llc. So I think you still need to 
> > set the options correct for llc.
> yes we have the set the options correct for llc in the code.
> 
> in the source file llvm/lib/CodeGen/CommandFlags.cpp, we have (in the patch 
> https://reviews.llvm.org/D87451 add new option -mignore-xcoff-visibility) , 
> the function
> TargetOptions codegen::InitTargetOptionsFromCodeGenFlags() {
> ....
> Options.IgnoreXCOFFVisibility = getIgnoreXCOFFVisibility(); 
> ...}
> 
What I'm saying is... 
I think we need a line like this:
`Options.IgnoreXCOFFVisibility = LangOpts.IgnoreXCOFFVisibility;`
so that when you invoke clang, backend would get the correct setting as well. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89986/new/

https://reviews.llvm.org/D89986

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

Reply via email to