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


================
Comment at: clang/docs/ClangCommandLineReference.rst:2622
+
+Do not emit any visibility attribute for asm on AIX or give all symbols 
'unspecified' visibility in xcoff object file(XCOFF only)
+
----------------
daltenty wrote:
> daltenty wrote:
> > nit: add a space before parens
> I don't think the object file writing case was handled yet? This makes it 
> sound like it is.
yes, we do not implement the visibility of the object file in the 
XCoffObjectWriter.cpp


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5242
 
+  if (const Arg *A = Args.getLastArg(options::OPT_mignore_xcoff_visibility)) {
+    if (Triple.isOSAIX())
----------------
daltenty wrote:
> Use `Args.hasFlag` instead, since this option doesn't have a value we need to 
> check.
we need to get the value here 
     D.Diag(diag::err_drv_unsupported_opt_for_target)
          << A->getAsString(Args) << TripleStr;


================
Comment at: clang/test/CodeGen/aix-ignore-xcoff-visibility.cpp:72
+
+// ERROR:         unsupported option '-mignore-xcoff-visibility' for target 
'powerpc-unknown-linux'
+
----------------
daltenty wrote:
> This isn't being checked anymore, also probably belongs in the other file
thanks


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

Reply via email to