DiggerLin marked 2 inline comments as done.
DiggerLin added inline comments.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:520
   Options.DataSections = CodeGenOpts.DataSections;
-  Options.IgnoreXCOFFVisibility = CodeGenOpts.IgnoreXCOFFVisibility;
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
----------------
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(); 
...}



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