[PATCH] D87451: add new clang option -mno-xcoff-visibility

[Jason Liu via Phabricator via llvm-commits]

jasonliu added inline comments.


================
Comment at: clang/docs/ClangCommandLineReference.rst:2310
 
+.. option:: -mno-xcoff-visibility
+
----------------
It's rare to see an option with only the negative form. Could we rename and make it a positive form somehow?
Also we would need to move this option to where the m group belongs. 



================
Comment at: clang/test/CodeGen/aix-no-xcoff-visibility.cpp:1
+// RUN: %clang -target powerpc-unknown-aix -emit-llvm -o - -S  %s  |\
+// RUN:   FileCheck --check-prefix=VISIBILITY-IR %s
----------------
I don't think we should call the driver directly in here. 
We should have a separate driver test where we invoke `clang`, and we should invoke the front end `clang_cc1` here.


================
Comment at: clang/test/CodeGen/aix-no-xcoff-visibility.cpp:75
+
+// VISIBILITY-IR:    @b = protected global i32 0
+// VISIBILITY-IR:    @pramb = hidden global i32 0
----------------
Not sure if the IR check is really necessary, since we haven't made any IR change here. It's going to be all the same with or without the new -m option. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87451