Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release/19.x: [AIX] Revert #pragma mc_func check (#102919) #102968

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Aug 12, 2024

Backport 123b6fc

Requested by: @qiongsiwu

@llvmbot llvmbot added this to the LLVM 19.X Release milestone Aug 12, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Aug 12, 2024

@AaronBallman What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from AaronBallman August 12, 2024 20:06
@llvmbot 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
@llvmbot
Copy link
Member Author

llvmbot commented Aug 12, 2024

@llvm/pr-subscribers-backend-powerpc

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

Backport 123b6fc

Requested by: @qiongsiwu


Full diff: https://github.com/llvm/llvm-project/pull/102968.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 5e412cb5f51894..15f9ee75492e3f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8090,13 +8090,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 35bb1a19d40f0a..f256d603ae6268 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 fb780fb75651d2..b04502a57a9f7a 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -557,12 +557,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]

@llvmbot
Copy link
Member Author

llvmbot commented Aug 12, 2024

@llvm/pr-subscribers-clang-driver

Author: None (llvmbot)

Changes

Backport 123b6fc

Requested by: @qiongsiwu


Full diff: https://github.com/llvm/llvm-project/pull/102968.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 5e412cb5f51894..15f9ee75492e3f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8090,13 +8090,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 35bb1a19d40f0a..f256d603ae6268 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 fb780fb75651d2..b04502a57a9f7a 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -557,12 +557,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]

Copy link

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

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)
@tru tru merged commit 28f2d04 into llvm:release/19.x Aug 13, 2024
3 of 5 checks passed
Copy link

@qiongsiwu (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:PowerPC clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

4 participants