[PATCH] D87451: add new option ignore-xcoff-visibility
David Tenty via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 2 10:11:47 PDT 2020
daltenty 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)
+
----------------
nit: add a space before parens
================
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:
> 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.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5242
+ if (const Arg *A = Args.getLastArg(options::OPT_mignore_xcoff_visibility)) {
+ if (Triple.isOSAIX())
----------------
Use `Args.hasFlag` instead, since this option doesn't have a value we need to check.
================
Comment at: clang/test/CodeGen/aix-ignore-xcoff-visibility.cpp:72
+
+// ERROR: unsupported option '-mignore-xcoff-visibility' for target 'powerpc-unknown-linux'
+
----------------
This isn't being checked anymore, also probably belongs in the other file
================
Comment at: clang/test/Driver/ignore-xcoff-visibility.cpp:2
+// RUN: %clang -### -target powerpc-unknown-aix -mignore-xcoff-visibility -S %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK %s < %t.log
+// CHECK: "-mignore-xcoff-visibility"
----------------
We should check the diagnostic here
================
Comment at: clang/test/Driver/ignore-xcoff-visibility.cpp:3
+// RUN: FileCheck -check-prefix=CHECK %s < %t.log
+// CHECK: "-mignore-xcoff-visibility"
----------------
nit: We should constrain this to be following the cc1 invocation
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87451/new/
https://reviews.llvm.org/D87451
More information about the cfe-commits
mailing list