[AIX] Revert `#pragma mc_func` check #102919

qiongsiwu · 2024-08-12T15:40:36Z

#99888 added a specific diagnostic for #pragma mc_func on AIX. There are some disagreements on:

If the check should be on by default. Leaving the check off by default is dangerous, since it is difficult to be aware of such a check. Turning it on by default at the moment causes build failures on AIX. See [AIX] Turn on #pragma mc_func check by default #101336 for more details.
If the check can be made more general. See [AIX] Turn on #pragma mc_func check by default #101336 (comment).

This PR reverts this check from main so we can flush out these disagreements.

This reverts commit b830790.

This reverts commit ee07547.

This reverts commit 9147147.

llvmbot · 2024-08-12T15:41:08Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Qiongsi Wu (qiongsiwu)

Changes

#99888 added a specific diagnostic for #pragma mc_func on AIX. There are some disagreements on:

If the check should be on by default. Leaving the check off by default is dangerous, since it is difficult to be aware of such a check. Turning it on by default at the moment causes build failures on AIX. See [AIX] Turn on #pragma mc_func check by default #101336 for more details.
If the check can be made more general. See [AIX] Turn on #pragma mc_func check by default #101336 (comment).

This PR reverts this check from main so we can flush out these disagreements.

Full diff: https://github.com/llvm/llvm-project/pull/102919.diff

7 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticParseKinds.td (-3)
(modified) clang/include/clang/Driver/Options.td (-7)
(modified) clang/include/clang/Lex/PreprocessorOptions.h (-5)
(modified) clang/include/clang/Parse/Parser.h (-1)
(modified) clang/lib/Driver/ToolChains/AIX.cpp (-6)
(modified) clang/lib/Parse/ParsePragma.cpp (-25)
(removed) clang/test/Preprocessor/pragma_mc_func.c (-23)

diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index f8d50d12bb9351..12aab09f285567 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1260,9 +1260,6 @@ def warn_pragma_intrinsic_builtin : Warning<
 def warn_pragma_unused_expected_var : Warning<
   "expected '#pragma unused' argument to be a variable name">,
   InGroup<IgnoredPragmas>;
-// - #pragma mc_func
-def err_pragma_mc_func_not_supported :
-   Error<"#pragma mc_func is not supported">;
 // - #pragma init_seg
 def warn_pragma_init_seg_unsupported_target : Warning<
   "'#pragma init_seg' is only supported when targeting a "
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index e196c3dc5cb3be..0b38139bd27972 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8114,13 +8114,6 @@ def source_date_epoch : Separate<["-"], "source-date-epoch">,
 
 } // let Visibility = [CC1Option]
 
-defm err_pragma_mc_func_aix : BoolFOption<"err-pragma-mc-func-aix",
-  PreprocessorOpts<"ErrorOnPragmaMcfuncOnAIX">, DefaultFalse,
-  PosFlag<SetTrue, [], [ClangOption, CC1Option],
-          "Treat uses of #pragma mc_func as errors">,
-  NegFlag<SetFalse,[], [ClangOption, CC1Option],
-          "Ignore uses of #pragma mc_func">>;
-
 //===----------------------------------------------------------------------===//
 // CUDA Options
 //===----------------------------------------------------------------------===//
diff --git a/clang/include/clang/Lex/PreprocessorOptions.h b/clang/include/clang/Lex/PreprocessorOptions.h
index 3f7dd9db18ba7d..c2e3d68333024a 100644
--- a/clang/include/clang/Lex/PreprocessorOptions.h
+++ b/clang/include/clang/Lex/PreprocessorOptions.h
@@ -211,10 +211,6 @@ class PreprocessorOptions {
   /// If set, the UNIX timestamp specified by SOURCE_DATE_EPOCH.
   std::optional<uint64_t> SourceDateEpoch;
 
-  /// If set, the preprocessor reports an error when processing #pragma mc_func
-  /// on AIX.
-  bool ErrorOnPragmaMcfuncOnAIX = false;
-
 public:
   PreprocessorOptions() : PrecompiledPreambleBytes(0, false) {}
 
@@ -252,7 +248,6 @@ class PreprocessorOptions {
     PrecompiledPreambleBytes.first = 0;
     PrecompiledPreambleBytes.second = false;
     RetainExcludedConditionalBlocks = false;
-    ErrorOnPragmaMcfuncOnAIX = false;
   }
 };
 
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 39c5f588167ede..99a0b0200fa06f 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -221,7 +221,6 @@ class Parser : public CodeCompletionHandler {
   std::unique_ptr<PragmaHandler> MaxTokensHerePragmaHandler;
   std::unique_ptr<PragmaHandler> MaxTokensTotalPragmaHandler;
   std::unique_ptr<PragmaHandler> RISCVPragmaHandler;
-  std::unique_ptr<PragmaHandler> MCFuncPragmaHandler;
 
   std::unique_ptr<CommentHandler> CommentSemaHandler;
 
diff --git a/clang/lib/Driver/ToolChains/AIX.cpp b/clang/lib/Driver/ToolChains/AIX.cpp
index b2885b7776d132..c2de7328c25c5d 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -560,12 +560,6 @@ void AIX::addClangTargetOptions(
   if (!Args.getLastArgNoClaim(options::OPT_fsized_deallocation,
                               options::OPT_fno_sized_deallocation))
     CC1Args.push_back("-fno-sized-deallocation");
-
-  if (Args.hasFlag(options::OPT_ferr_pragma_mc_func_aix,
-                   options::OPT_fno_err_pragma_mc_func_aix, false))
-    CC1Args.push_back("-ferr-pragma-mc-func-aix");
-  else
-    CC1Args.push_back("-fno-err-pragma-mc-func-aix");
 }
 
 void AIX::addProfileRTLibs(const llvm::opt::ArgList &Args,
diff --git a/clang/lib/Parse/ParsePragma.cpp b/clang/lib/Parse/ParsePragma.cpp
index aef4ddb7588164..cc6f18b5b319f9 100644
--- a/clang/lib/Parse/ParsePragma.cpp
+++ b/clang/lib/Parse/ParsePragma.cpp
@@ -14,7 +14,6 @@
 #include "clang/Basic/PragmaKinds.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Preprocessor.h"
-#include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Lex/Token.h"
 #include "clang/Parse/LoopHint.h"
 #include "clang/Parse/ParseDiagnostic.h"
@@ -412,19 +411,6 @@ struct PragmaRISCVHandler : public PragmaHandler {
   Sema &Actions;
 };
 
-struct PragmaMCFuncHandler : public PragmaHandler {
-  PragmaMCFuncHandler(bool ReportError)
-      : PragmaHandler("mc_func"), ReportError(ReportError) {}
-  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
-                    Token &Tok) override {
-    if (ReportError)
-      PP.Diag(Tok, diag::err_pragma_mc_func_not_supported);
-  }
-
-private:
-  bool ReportError = false;
-};
-
 void markAsReinjectedForRelexing(llvm::MutableArrayRef<clang::Token> Toks) {
   for (auto &T : Toks)
     T.setFlag(clang::Token::IsReinjected);
@@ -582,12 +568,6 @@ void Parser::initializePragmaHandlers() {
     RISCVPragmaHandler = std::make_unique<PragmaRISCVHandler>(Actions);
     PP.AddPragmaHandler("clang", RISCVPragmaHandler.get());
   }
-
-  if (getTargetInfo().getTriple().isOSAIX()) {
-    MCFuncPragmaHandler = std::make_unique<PragmaMCFuncHandler>(
-        PP.getPreprocessorOpts().ErrorOnPragmaMcfuncOnAIX);
-    PP.AddPragmaHandler(MCFuncPragmaHandler.get());
-  }
 }
 
 void Parser::resetPragmaHandlers() {
@@ -722,11 +702,6 @@ void Parser::resetPragmaHandlers() {
     PP.RemovePragmaHandler("clang", RISCVPragmaHandler.get());
     RISCVPragmaHandler.reset();
   }
-
-  if (getTargetInfo().getTriple().isOSAIX()) {
-    PP.RemovePragmaHandler(MCFuncPragmaHandler.get());
-    MCFuncPragmaHandler.reset();
-  }
 }
 
 /// Handle the annotation token produced for #pragma unused(...)
diff --git a/clang/test/Preprocessor/pragma_mc_func.c b/clang/test/Preprocessor/pragma_mc_func.c
deleted file mode 100644
index f0d3e49e5dddca..00000000000000
--- a/clang/test/Preprocessor/pragma_mc_func.c
+++ /dev/null
@@ -1,23 +0,0 @@
-// RUN: not %clang --target=powerpc64-ibm-aix -ferr-pragma-mc-func-aix -fsyntax-only \
-// RUN:   %s 2>&1 | FileCheck %s
-#pragma mc_func asm_barrier {"60000000"}
-
-// CHECK:  error: #pragma mc_func is not supported
-
-// Cases where no errors occur.
-// RUN: %clang --target=powerpc64-ibm-aix -fno-err-pragma-mc-func-aix -fsyntax-only %s
-// RUN: %clang --target=powerpc64-ibm-aix -ferr-pragma-mc-func-aix -fsyntax-only \
-// RUN:    -fno-err-pragma-mc-func-aix %s
-// RUN: %clang --target=powerpc64-ibm-aix -fsyntax-only %s
-// RUN: %clang --target=powerpc64-ibm-aix -Werror=unknown-pragmas \
-// RUN:   -fno-err-pragma-mc-func-aix -fsyntax-only %s
-
-// Cases where we have errors or warnings.
-// RUN: not %clang --target=powerpc64le-unknown-linux-gnu \
-// RUN:   -Werror=unknown-pragmas -fno-err-pragma-mc-func-aix -fsyntax-only %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=UNUSED %s
-// RUN: %clang --target=powerpc64le-unknown-linux-gnu \
-// RUN:   -fno-err-pragma-mc-func-aix -fsyntax-only %s 2>&1 | \
-// RUN:   FileCheck --check-prefix=UNUSED %s
-
-// UNUSED: clang: warning: argument unused during compilation: '-fno-err-pragma-mc-func-aix' [-Wunused-command-line-argument]

AaronBallman

LGTM, thank you!

qiongsiwu · 2024-08-12T20:01:19Z

/cherry-pick 123b6fc

llvm#99888 added a specific diagnostic for `#pragma mc_func` on AIX. There are some disagreements on: 1. If the check should be on by default. Leaving the check off by default is dangerous, since it is difficult to be aware of such a check. Turning it on by default at the moment causes build failures on AIX. See llvm#101336 for more details. 2. If the check can be made more general. See llvm#101336 (comment). This PR reverts this check from `main` so we can flush out these disagreements. (cherry picked from commit 123b6fc)

llvmbot · 2024-08-12T20:06:18Z

/pull-request #102968

llvm#99888 added a specific diagnostic for `#pragma mc_func` on AIX. There are some disagreements on: 1. If the check should be on by default. Leaving the check off by default is dangerous, since it is difficult to be aware of such a check. Turning it on by default at the moment causes build failures on AIX. See llvm#101336 for more details. 2. If the check can be made more general. See llvm#101336 (comment). This PR reverts this check from `main` so we can flush out these disagreements. (cherry picked from commit 123b6fc)

llvm#99888 added a specific diagnostic for `#pragma mc_func` on AIX. There are some disagreements on: 1. If the check should be on by default. Leaving the check off by default is dangerous, since it is difficult to be aware of such a check. Turning it on by default at the moment causes build failures on AIX. See llvm#101336 for more details. 2. If the check can be made more general. See llvm#101336 (comment). This PR reverts this check from `main` so we can flush out these disagreements.

Qiongsi Wu added 3 commits August 12, 2024 11:34

Revert "[NFC] changes all run lines"

acf020c

This reverts commit b830790.

Revert "[NFC] make the case only require frontend"

f759d3e

This reverts commit ee07547.

Revert "[AIX] Detect #pragma mc_func (llvm#99888)"

205d2c8

This reverts commit 9147147.

qiongsiwu requested review from chenzheng1030 and AaronBallman August 12, 2024 15:40

llvmbot added clang Clang issues not falling into any other category backend:PowerPC clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Aug 12, 2024

qiongsiwu mentioned this pull request Aug 12, 2024

[AIX] Turn on #pragma mc_func check by default #101336

Merged

qiongsiwu added this to the LLVM 19.X Release milestone Aug 12, 2024

AaronBallman approved these changes Aug 12, 2024

View reviewed changes

qiongsiwu merged commit 123b6fc into llvm:main Aug 12, 2024
13 checks passed

mgabka mentioned this pull request Aug 22, 2024

Add release note about ABI mgabka/llvm-project#6

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AIX] Revert `#pragma mc_func` check #102919

[AIX] Revert `#pragma mc_func` check #102919

qiongsiwu commented Aug 12, 2024

llvmbot commented Aug 12, 2024 •

edited

Loading

AaronBallman left a comment

qiongsiwu commented Aug 12, 2024

llvmbot commented Aug 12, 2024

[AIX] Revert #pragma mc_func check #102919

[AIX] Revert #pragma mc_func check #102919

Conversation

qiongsiwu commented Aug 12, 2024

llvmbot commented Aug 12, 2024 • edited Loading

AaronBallman left a comment

Choose a reason for hiding this comment

qiongsiwu commented Aug 12, 2024

llvmbot commented Aug 12, 2024

[AIX] Revert `#pragma mc_func` check #102919

[AIX] Revert `#pragma mc_func` check #102919

llvmbot commented Aug 12, 2024 •

edited

Loading