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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
