Addressed the review comments. removed redundant tests

Author: Tony Varghese

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -1369,7 +1369,7 @@ def err_pragma_comment_unknown_kind : Error<"unknown kind of pragma comment">;
 def warn_pragma_comment_ignored : Warning<"'#pragma comment %0' ignored">,
   InGroup<IgnoredPragmas>;
 def warn_pragma_comment_once : Warning<"'#pragma comment %0' "
-  "can be specified only once per source file - ignored">,
+  "can be specified only once per translation unit - ignored">,
   InGroup<IgnoredPragmas>;
 // - #pragma detect_mismatch
 def err_pragma_detect_mismatch_malformed : Error<

clang/lib/CodeGen/CodeGenModule.cpp

@@ -1570,11 +1570,9 @@ void CodeGenModule::Release() {
   EmitBackendOptionsMetadata(getCodeGenOpts());
 
   // Emit copyright metadata for AIX
-  if (!AIXCopyrightComment.empty()) {
-    auto *NMD =
-        getModule().getOrInsertNamedMetadata("aix.copyright.comment");
-    for (auto *MD : AIXCopyrightComment)
-      NMD->addOperand(MD);
+  if (AIXCopyrightComment) {
+    auto *NMD = getModule().getOrInsertNamedMetadata("aix.copyright.comment");
+    NMD->addOperand(AIXCopyrightComment);
   }
 
   // If there is device offloading code embed it in the host now.
@@ -3410,21 +3408,19 @@ void CodeGenModule::AddDependentLib(StringRef Lib) {
   LinkerOptionsMetadata.push_back(llvm::MDNode::get(C, MDOpts));
 }
 
-/// Process the #pragma comment(copyright, " copy right string ")
-/// and create llvm metadata for the copyrgiht
+/// Process the #pragma comment(copyright, "copyright string ")
+/// and create llvm metadata for the copyright
 void CodeGenModule::ProcessPragmaCommentCopyright(StringRef Comment) {
 
-  // Pragma Comment Copyright is enabled only when:
-  //  - OS is AIX
-  //  - Comment is non empty
-  if (!getTriple().isOSAIX() || Comment.empty())
+  if (!getTriple().isOSAIX())
     return;
 
   // Create llvm metadata with the comment string
   auto &C = getLLVMContext();
   llvm::Metadata *Ops[] = {llvm::MDString::get(C, Comment.str())};
   auto *Node = llvm::MDNode::get(C, Ops);
-  AIXCopyrightComment.push_back(Node); // This should be available during the runtime
+  if(!AIXCopyrightComment)
+    AIXCopyrightComment = Node;
 }
 
 /// Add link options implied by the given module, including modules

clang/lib/CodeGen/CodeGenModule.h

@@ -587,8 +587,9 @@ class CodeGenModule : public CodeGenTypeCache {
   /// A vector of metadata strings for dependent libraries for ELF.
   SmallVector<llvm::MDNode *, 16> ELFDependentLibraries;
 
-  /// A vector of metadata strings for copyright comment for AIX
-  SmallVector<llvm::MDNode *, 16> AIXCopyrightComment;
+  /// Single module-level copyright comment for AIX (if any).
+  /// We only ever accept one per TU.
+  llvm::MDNode *AIXCopyrightComment = nullptr;
 
   /// @name Cache for Objective-C runtime types
   /// @{
@@ -1461,7 +1462,7 @@ class CodeGenModule : public CodeGenTypeCache {
   /// Appends a dependent lib to the appropriate metadata value.
   void AddDependentLib(StringRef Lib);
 
-  /// Append AIX copyright comment to the module-level metadata.
+  /// Record the AIX copyright comment in module-level metadata.
   void ProcessPragmaCommentCopyright(StringRef Comment); 
 
   llvm::GlobalVariable::LinkageTypes getFunctionLinkage(GlobalDecl GD);

clang/lib/Parse/ParsePragma.cpp

@@ -236,6 +236,7 @@ struct PragmaCommentHandler : public PragmaHandler {
 
 private:
   Sema &Actions;
+  bool SeenAIXCopyright = false; // TU-scoped
 };
 
 struct PragmaDetectMismatchHandler : public PragmaHandler {
@@ -3223,17 +3224,14 @@ void PragmaCommentHandler::HandlePragma(Preprocessor &PP,
     return;
   }
 
-  // pragma comment copyright can each appear only once in a TU.
-  if (PP.getTargetInfo().getTriple().isOSAIX()) {
-    static bool SeenAIXCopyright = false;
-    if (Kind == PCK_Copyright) {
+  // On AIX, pragma comment copyright can each appear only once in a TU.
+  if (PP.getTargetInfo().getTriple().isOSAIX() && Kind == PCK_Copyright) {
       if (SeenAIXCopyright) {
         PP.Diag(Tok.getLocation(), diag::warn_pragma_comment_once)
             << II->getName();
         return;
       }
       SeenAIXCopyright = true;
-    }
   }
 
   // Read the optional string if present.

clang/test/CodeGen/PowerPC/pragma-comment-copyright-aix-multi-lto.c

@@ -1,4 +1,3 @@
-// REQUIRES: powerpc-registered-target, system-aix
 // RUN: %clang_cc1 %s -triple powerpc-ibm-aix -O0 -disable-llvm-passes -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 %s -triple powerpc64-ibm-aix -O0 -disable-llvm-passes -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 %s -triple powerpc-ibm-aix -verify
@@ -16,7 +15,7 @@
 #pragma comment(copyright, "@(#) Copyright")
 
 // Test duplicate copyright - should warn and ignore
-#pragma comment(copyright, "Duplicate Copyright") // expected-warning {{'#pragma comment copyright' can be specified only once per source file - ignored}}
+#pragma comment(copyright, "Duplicate Copyright") // expected-warning {{'#pragma comment copyright' can be specified only once per translation unit - ignored}}
 
 int main() { return 0; }
 

clang/test/CodeGen/PowerPC/pragma-comment-copyright-aix.c

@@ -5,24 +5,6 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-//
-// The CopyrightMetadataPass lowers the module-level metadata emitted by Clang
-// for `#pragma comment(copyright, "...")` on AIX:
-//
-//     !aix.copyright.comment = !{!"Copyright ..."}
-//
-// into an internal constant string global that is preserved across all compiler
-// and linker stages. Each translation unit produces one TU-local string symbol
-// (`__aix_copyright_str`), and the pass attaches `!implicit.ref` metadata to
-// defined functions referencing this symbol. The PowerPC AIX backend recognizes
-// this metadata and emits `.ref` directives in the XCOFF assembly, ensuring the
-// copyright strings:
-//
-//   • survive optimization and LTO,
-//   • are not removed by linker garbage collection, and
-//   • remain visible in the final binary.
-//
-//===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TRANSFORMS_UTILS_COPYRIGHTMETADATAPASS_H
 #define LLVM_TRANSFORMS_UTILS_COPYRIGHTMETADATAPASS_H
@@ -40,4 +22,4 @@ class CopyrightMetadataPass : public PassInfoMixin<CopyrightMetadataPass> {
 
 } // namespace llvm
 
-#endif // LLVM_TRANSFORMS_UTILS_WYVERN_COPYRIGHTMETADATAPASS_H
+#endif // LLVM_TRANSFORMS_UTILS_COPYRIGHTMETADATAPASS_H

llvm/include/llvm/Transforms/Utils/CopyrightMetadataPass.h

@@ -6,21 +6,22 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// This pass lowers module-level copyright metadata emitted by Clang:
+// CopyrightMetadataPass pass lowers module-level copyright metadata emitted by
+// Clang:
 //
 //     !aix.copyright.comment = !{!"Copyright ..."}
 //
 // into concrete, translation-unit–local globals to ensure that copyright
 // strings:
-//
-//   • survive all optimization and LTO pipelines,
-//   • are not removed by linker garbage collection, and
-//   • remain visible in the final XCOFF binary.
+//   - survive all optimization and LTO pipelines,
+//   - are not removed by linker garbage collection, and
+//   - remain visible in the final XCOFF binary.
 //
 // For each module (translation unit), the pass performs the following:
 //
 //   1. Creates a null-terminated, internal constant string global
-//      (`__aix_copyright_str`) containing the copyright text.
+//      (`__aix_copyright_str`) containing the copyright text in
+//      `__aix_copyright` section..
 //
 //   2. Marks the string in `llvm.used` so it cannot be dropped by
 //      optimization or LTO.
@@ -29,11 +30,25 @@
 //      defined function in the module. The PowerPC AIX backend recognizes
 //      this metadata and emits a `.ref` directive from the function to the
 //      string, creating a concrete relocation that prevents the linker from
-//      discarding it.
+//      discarding it (as long as the referencing symbol is kept).
+//
+//  Input IR:
+//     !aix.copyright.comment = !{!"Copyright"}
+//  Output IR:
+//     @__aix_copyright_str = internal constant [N x i8] c"Copyright\00",
+//                          section "__aix_copyright"
+//     @llvm.used = appending global [1 x ptr] [ptr @__aix_copyright_str]
 //
+//     define i32 @func() !implicit.ref !5 { ... }
+//     !5 = !{ptr @__aix_copyright_str}
+//
+// The copyright string is placed in the "__aix_copyright" section (mapped to
+// an XCOFF csect with [RO] storage class), making it easily identifiable in
+// object files and executables. The R_REF relocation prevents the linker
+// from discarding this section during garbage collection.  Copyright string (if
+// kept by the linker) is expected to be loaded at run time.
 //===----------------------------------------------------------------------===//
 
-
 #include "llvm/Transforms/Utils/CopyrightMetadataPass.h"
 
 #include "llvm/ADT/SmallVector.h"
@@ -44,7 +59,6 @@
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/GlobalVariable.h"
-#include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
@@ -60,8 +74,6 @@
 
 using namespace llvm;
 
-namespace llvm {
-
 static cl::opt<bool>
     DisableCopyrightMetadata("disable-copyright-metadata", cl::ReallyHidden,
                              cl::desc("Disable copyright metadata pass."),
@@ -106,22 +118,28 @@ PreservedAnalyses CopyrightMetadataPass::run(Module &M,
                                    /*isConstant=*/true,
                                    GlobalValue::InternalLinkage, StrInit,
                                    /*Name=*/"__aix_copyright_str");
+  // Set unnamed_addr to allow the linker to merge identical strings
   StrGV->setUnnamedAddr(GlobalValue::UnnamedAddr::Global);
   StrGV->setAlignment(Align(1));
-  StrGV->setSection("__llvm_copyright");
-
-  // 2. Ensure LLVM doesn't delete it (through all pipelines/LTO).
+  // Place in the "__aix_copyright" section.
+  // Backend maps this to an appropriate XCOFF csect (typically [RO])
+  // The section will appear in assembly as:
+  //   .csect __aix_copyright[RO],2
+  StrGV->setSection("__aix_copyright");
+
+  // 2. Add the string to llvm.used to prevent LLVM optimization/LTO passes from
+  // removing it.
   appendToUsed(M, {StrGV});
-  // appendToCompilerUsed(M, {StrGV});
 
   // 3. Attach !implicit ref to every defined function
   // Create a metadata node pointing to the copyright string:
   //   !N = !{ptr @__aix_copyright_str}
   Metadata *Ops[] = {ConstantAsMetadata::get(StrGV)};
-  auto *ValMD = ValueAsMetadata::get(StrGV);
   MDNode *ImplicitRefMD = MDNode::get(Ctx, Ops);
 
-  auto addImplicitRef = [&](Function &F) {
+  // Lambda to attach implicit.ref metadata to a function.
+  // The backend will translate this into .ref assembly directives.
+  auto AddImplicitRef = [&](Function &F) {
     if (F.isDeclaration())
       return;
     // Attach the implicit.ref metadata to the function
@@ -130,15 +148,13 @@ PreservedAnalyses CopyrightMetadataPass::run(Module &M,
                       << F.getName() << "\n");
   };
 
+  // Process all functions in the module
   for (Function &F : M)
-    addImplicitRef(F);
+    AddImplicitRef(F);
 
-  // Remove the original metadata since we've processed it
-  // This prevents reprocessing if the pass runs multiple times
+  // Cleanup the processed metadata.
   MD->eraseFromParent();
   LLVM_DEBUG(dbgs() << "[copyright] created string and anchor for module\n");
 
   return PreservedAnalyses::all();
 }
-
-} // namespace llvm