[PowerPC][AIX] Support #pragma comment copyright for AIX #1

tonykuttai · 2025-10-18T15:47:50Z

Add support for the #pragma comment (copyright, "Copyright info") directive for AIX.

This builds on top of PR #159096

Refernce: XL documentation for #pragma comment

llvm/test/Transforms/CopyrightMetadata/copyright-metadata.ll

clang/lib/Parse/ParsePragma.cpp

clang/lib/CodeGen/CodeGenModule.cpp

llvm/lib/Transforms/Utils/CopyrightMetadataPass.cpp

hubert-reinterpretcast · 2025-10-20T00:02:05Z

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

@@ -0,0 +1,110 @@
+// REQUIRES: powerpc-registered-target, system-aix, clang


I don't think this test fits with the LLVM unit testing philosophy.

My understanding is that we have tests

for the front-end generation of the aix.copyright.comment metadata, and

for the transformation of the aix.copyright.comment metadata to implicit.ref, etc.

What we do not have are targeted tests that specifically verify that the transformation is run at the right point in the various possible pipelines.

you are right. Removed this test.

What we do not have are targeted tests that specifically verify that the transformation is run at the right point in the various possible pipelines.

Added the opt levels testing to llvm/test/Transforms/CopyrightMetadata/copyright-metadata.ll. Do we need an end-to-end test ?

Do we need a end-to-end test ?

I can't say that we shouldn't have one. It is just that I am not aware of there being any in upstream LLVM that involve the linker (except when it's an lld test).

I couldn't find anything similar as well.

llvm/test/Transforms/CopyrightMetadata/copyright-metadata.ll

clang/lib/CodeGen/CodeGenModule.h

tonykuttai · 2025-10-22T12:09:40Z

Added the test clang/test/CodeGen/PowerPC/pragma-comment-copyright-aix-modules.cpp to test the C++ 20 Modules getting ignored in the importing TU.

hubert-reinterpretcast

Some partial review comments...

hubert-reinterpretcast · 2025-10-22T17:28:42Z

llvm/test/Transforms/CopyrightMetadata/copyright-metadata.ll

+; RUN: opt --O0 -S %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-O0
+; RUN: opt --O1 -S %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-ON
+; RUN: opt --O2 -S %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-ON
+; RUN: opt --O3 -S %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-ON


I'm not familiar enough with how the pipelines are built between different tools (clang, opt, etc.) to know if this is the right way to check that the pass is in the pipeline at the right place (especially for LTO).

@w2yehia @mandlebug, your input would be appreciated.

hubert-reinterpretcast · 2025-10-22T17:37:42Z

llvm/test/CodeGen/PowerPC/pragma-comment-copyright-aix.ll

+;   - Emits the string in a dedicated read-only csect
+;
+; CHECK-ASM:       .ref __aix_copyright_str
+; CHECK-ASM:       .csect __aix_copyright[RO],2


Sorry for the churn. The check in llvm/test/Transforms/CopyrightMetadata/copyright-metadata.ll for constant in the IR is sufficient for establishing that we are generating as read-only, so the testing request from #1 (comment) was already satisfied (and we don't need this test just to cover that).

It is still a question whether we want to group the strings into one csect (which probably has no effect except under "full" LTO where it will increase the chance of the string being kept).

ok, sure. Removing the llvm/test/CodeGen/PowerPC/pragma-comment-copyright-aix.ll.

It is still a question whether we want to group the strings into one csect (which probably has no effect except under "full" LTO where it will increase the chance of the string being kept).

Are there any downsides to the current approach ? I think it is cleaner to group them seperately in a csect.

Are there any downsides to the current approach ?

It generates "false positives" to the question of whether the content associated with the copyright string was actually retained/relevant to the result.

clang/lib/Parse/ParsePragma.cpp

clang/lib/CodeGen/CodeGenModule.cpp

clang/lib/CodeGen/CodeGenModule.h

clang/lib/CodeGen/CodeGenModule.cpp

clang/lib/CodeGen/CodeGenModule.h

clang/test/CodeGen/PowerPC/pragma-comment-copyright-aix-modules.cpp

clang/lib/CodeGen/CodeGenModule.cpp

clang/lib/CodeGen/CodeGenModule.h

clang/lib/Parse/ParsePragma.cpp

llvm/lib/Transforms/Utils/CopyrightMetadataPass.cpp

hubert-reinterpretcast

Please add updates to existing tests so that they continue to pass. For example, llvm/test/Other/new-pm-defaults.ll fails with:

# .---command stderr------------
# | /llvm/test/Other/new-pm-defaults.ll:241:23: error: CHECK-DEFAULT-NEXT: is not on the line after the previous match
# | ; CHECK-DEFAULT-NEXT: Running pass: EliminateAvailableExternallyPass
# |                       ^
# | <stdin>:100:1: note: 'next' match was here
# | Running pass: EliminateAvailableExternallyPass on [module]
# | ^
# | <stdin>:98:28: note: previous match ended here
# | Running pass: GlobalDCEPass on [module]
# |                            ^
# | <stdin>:99:1: note: non-matching line after previous match is here
# | Running pass: CopyrightMetadataPass on [module]
# | ^

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

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

llvm/test/Transforms/CopyrightMetadata/copyright-metadata.ll

…slation unit, add backend tests

- Fix test failures: Update pipeline tests for CopyrightMetadataPass - Fixed pragma comment copyright test expectations

…s.cpp to be an llvm based test

tonykuttai · 2025-12-05T06:01:40Z

Rebased the branch with latest updates from mandlebug:XCOFFAssociatedMetadata

perry-ca · 2025-12-05T15:35:46Z

llvm/lib/Passes/PassRegistry.def

 MODULE_PASS("constmerge", ConstantMergePass())
 MODULE_PASS("coro-cleanup", CoroCleanupPass())
 MODULE_PASS("coro-early", CoroEarlyPass())
+MODULE_PASS("copyright-metadata", CopyrightMetadataPass())


The name of this pass should be updated too so it is independent of the word "copyright". Nothing in the IR refers to copyright. To make it easier for people to understand, the pass name should reflect the IR meta data it is processing.

How about LowerCommentStringPass or LowerLoadTimeCommentPass?

perry-ca · 2025-12-05T15:37:26Z

llvm/lib/Transforms/Utils/CopyrightMetadataPass.cpp

+//  Input IR:
+//     !comment_string.loadtime = !{!"Copyright"}
+//  Output IR:
+//     @__loadtime_copyright_str = internal constant [N x i8] c"Copyright\00",


Rename this to __loadtime_comment_str.

perry-ca · 2025-12-05T15:38:02Z

llvm/lib/Transforms/Utils/CopyrightMetadataPass.cpp

+//     !comment_string.loadtime = !{!"Copyright"}
+//  Output IR:
+//     @__loadtime_copyright_str = internal constant [N x i8] c"Copyright\00",
+//                          section "__copyright_comment"


I assume this can be renamed too. Something like __loadtime_comment

hubert-reinterpretcast reviewed Oct 20, 2025

View reviewed changes

tonykuttai force-pushed the tvarghese/pragma-comment-copyright branch from 438f04c to cfa6019 Compare October 21, 2025 14:22

hubert-reinterpretcast reviewed Oct 21, 2025

View reviewed changes

llvm/test/Transforms/CopyrightMetadata/copyright-metadata.ll Outdated Show resolved Hide resolved

clang/lib/CodeGen/CodeGenModule.h Outdated Show resolved Hide resolved

tonykuttai force-pushed the tvarghese/pragma-comment-copyright branch from 252f063 to 82254ee Compare October 22, 2025 11:58

hubert-reinterpretcast reviewed Oct 22, 2025

View reviewed changes

perry-ca reviewed Oct 29, 2025

View reviewed changes

tonykuttai requested review from hubert-reinterpretcast and perry-ca October 29, 2025 07:19

hubert-reinterpretcast reviewed Oct 29, 2025

View reviewed changes

hubert-reinterpretcast reviewed Nov 12, 2025

View reviewed changes

perry-ca reviewed Dec 3, 2025

View reviewed changes

clang/test/CodeGen/PowerPC/pragma-comment-copyright-aix.c Outdated Show resolved Hide resolved

llvm/include/llvm/Transforms/Utils/CopyrightMetadataPass.h Outdated Show resolved Hide resolved

llvm/test/Transforms/CopyrightMetadata/copyright-metadata.ll Outdated Show resolved Hide resolved

mandlebug force-pushed the XCOFFAssociatedMetadata branch from 325741a to 4d6f365 Compare December 3, 2025 20:37

Tony Varghese added 6 commits December 4, 2025 03:13

[PowerPC][AIX] Support #pragma comment copyright for AIX

d216a07

Addressed the review comments. removed redundant tests

ed469d4

Pragmas from imported C++20 modules are ignored in the importing tran…

f47b0d4

…slation unit, add backend tests

Chnaged to platform agnostic names for llvm metadata and variables used

696ca9f

Changed the name to comment_string.loadtime

c21153e

- Fix test failures: Update pipeline tests for CopyrightMetadataPass - Fixed pragma comment copyright test expectations

Update clang/test/CodeGen/PowerPC/pragma-comment-copyright-aix-module…

a2904ba

…s.cpp to be an llvm based test

tonykuttai force-pushed the tvarghese/pragma-comment-copyright branch from 19a0c93 to a2904ba Compare December 5, 2025 05:55

perry-ca reviewed Dec 5, 2025

View reviewed changes

Changed the name of the pass to LowerCommentStringPass

0402627

		@@ -0,0 +1,110 @@
		// REQUIRES: powerpc-registered-target, system-aix, clang

[PowerPC][AIX] Support #pragma comment copyright for AIX #1

Are you sure you want to change the base?

[PowerPC][AIX] Support #pragma comment copyright for AIX #1

Uh oh!

Conversation

tonykuttai commented Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonykuttai Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tonykuttai commented Oct 22, 2025

Uh oh!

hubert-reinterpretcast left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hubert-reinterpretcast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tonykuttai commented Dec 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonykuttai Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonykuttai commented Oct 18, 2025 •

edited

Loading

tonykuttai Oct 21, 2025 •

edited

Loading

tonykuttai Dec 5, 2025 •

edited

Loading