DiggerLin marked an inline comment 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; ---------------- DiggerLin wrote: > jasonliu wrote: > > 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. > I do not think so, from the clang FE, we do not generated the visibility in > the IR. so there is no need these line. or we can say that because we do not set the hidden visibility into the GlobalValue , so we do not need the Options.IgnoreXCOFFVisibility = LangOpts.IgnoreXCOFFVisibility; 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
