Skip to content

Conversation

@aabhinavg1
Copy link
Contributor

Fixes #130269

Testing:

  • command
ninja check-llvm-codegen-powerpc
===== Before Patch =====

[779/780] Running lit suite /llvm/test/CodeGen/PowerPC
Testing Time: 44.38s
Total Discovered Tests: 1952
  Unsupported      :   38 (1.95%)
  Passed           : 1907 (97.69%)
  Expectedly Failed:    7 (0.36%)
===== After Patch =====
[779/780] Running lit suite /llvm/test/CodeGen/PowerPC
Testing Time: 120.31s
Total Discovered Tests: 1952
  Unsupported      :    3 (0.15%)
  Passed           : 1941 (99.44%)
  Expectedly Failed:    8 (0.41%)

Verification:

$ ../bin/clang -target powerpc64-ibm-aix -S test.c -o test.s
$ cat test.s

    .file   "test.c",,"LLVM version 22.0.0git ..."
    .csect ..text..[PR],5
    .rename ..text..[PR],""
    # Start of file scope inline assembly
    .weak   foo
foo:
    blr
    # End of file scope inline assembly
    .machine    "PWR7"

Input C code:

__asm__(".weak_definition foo\n"
        "foo:\n"
        "blr");

Outcome:

  • .weak_definition in the source is correctly mapped to .weak in the assembly on AIX.
  • Confirms the patch fixes the AIX assembler issue without affecting other targets.

- On AIX, the assembler does not recognize '.weak_definition'.
- Emit MCSA_WeakDefinition symbols using the standard weak directive (.weak)
  and set storage class to C_WEAKEXT in XCOFF.
- Added test llvm/test/MC/PowerPC/aix-weak-definition.s
@aabhinavg1 aabhinavg1 requested a review from tonykuttai August 29, 2025 17:42
@aabhinavg1 aabhinavg1 self-assigned this Aug 29, 2025
@llvmbot llvmbot added backend:PowerPC llvm:mc Machine (object) code labels Aug 29, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 29, 2025

@llvm/pr-subscribers-backend-powerpc

Author: None (aabhinavg1)

Changes
  • On AIX, the assembler does not recognize '.weak_definition'.
  • Emit MCSA_WeakDefinition symbols using the standard weak directive (.weak) and set storage class to C_WEAKEXT in XCOFF.
  • Added test llvm/test/MC/PowerPC/aix-weak-definition.s
    Fixes #130269

Fixes #130269

Testing:

  • command
ninja check-llvm-codegen-powerpc
===== Before Patch =====

[779/780] Running lit suite /llvm/test/CodeGen/PowerPC
Testing Time: 44.38s
Total Discovered Tests: 1952
  Unsupported      :   38 (1.95%)
  Passed           : 1907 (97.69%)
  Expectedly Failed:    7 (0.36%)
===== After Patch =====
[779/780] Running lit suite /llvm/test/CodeGen/PowerPC
Testing Time: 120.31s
Total Discovered Tests: 1952
  Unsupported      :    3 (0.15%)
  Passed           : 1941 (99.44%)
  Expectedly Failed:    8 (0.41%)

Verification:

$ ../bin/clang -target powerpc64-ibm-aix -S test.c -o test.s
$ cat test.s

    .file   "test.c",,"LLVM version 22.0.0git ..."
    .csect ..text..[PR],5
    .rename ..text..[PR],""
    # Start of file scope inline assembly
    .weak   foo
foo:
    blr
    # End of file scope inline assembly
    .machine    "PWR7"

Input C code:

__asm__(".weak_definition foo\n"
        "foo:\n"
        "blr");

Outcome:

  • .weak_definition in the source is correctly mapped to .weak in the assembly on AIX.
  • Confirms the patch fixes the AIX assembler issue without affecting other targets.

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

3 Files Affected:

  • (modified) llvm/lib/MC/MCAsmStreamer.cpp (+11-3)
  • (modified) llvm/lib/MC/MCXCOFFStreamer.cpp (+7)
  • (added) llvm/test/MC/PowerPC/aix-weak-definition.s (+13)
diff --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp
index be8c022f39ad1..7b1a1878915bc 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -764,10 +764,18 @@ bool MCAsmStreamer::emitSymbolAttribute(MCSymbol *Symbol,
     OS << "\t.extern\t";
     break;
   case MCSA_Weak:           OS << MAI->getWeakDirective(); break;
-  case MCSA_WeakDefinition:
-    OS << "\t.weak_definition\t";
+  case MCSA_WeakDefinition: {
+    // AIX, use the standard weak directive (.weak) instead of
+    //'.weak_definition' because the AIX assembler does not
+    // recognize the '.weak_definition' directive.
+    const llvm::Triple &TT = getContext().getTargetTriple();
+    if (TT.isOSAIX())
+      OS << MAI->getWeakDirective();
+    else
+      OS << "\t.weak_definition\t";
     break;
-      // .weak_reference
+  }
+    // .weak_reference
   case MCSA_WeakReference:  OS << MAI->getWeakRefDirective(); break;
   case MCSA_WeakDefAutoPrivate: OS << "\t.weak_def_can_be_hidden\t"; break;
   case MCSA_Cold:
diff --git a/llvm/lib/MC/MCXCOFFStreamer.cpp b/llvm/lib/MC/MCXCOFFStreamer.cpp
index 4bf14c11068cb..cd5398dc7bc3b 100644
--- a/llvm/lib/MC/MCXCOFFStreamer.cpp
+++ b/llvm/lib/MC/MCXCOFFStreamer.cpp
@@ -72,6 +72,13 @@ bool MCXCOFFStreamer::emitSymbolAttribute(MCSymbol *Sym,
     Symbol->setStorageClass(XCOFF::C_WEAKEXT);
     Symbol->setExternal(true);
     break;
+  case llvm::MCSA_WeakDefinition:
+    // On AIX/XCOFF, a weak definition symbol should be emitted
+    // as an external weak symbol (C_WEAKEXT), since the assembler
+    // does not support '.weak_definition' directly.
+    Symbol->setStorageClass(XCOFF::C_WEAKEXT);
+    Symbol->setExternal(true);
+    break;
   case llvm::MCSA_Hidden:
     Symbol->setVisibilityType(XCOFF::SYM_V_HIDDEN);
     break;
diff --git a/llvm/test/MC/PowerPC/aix-weak-definition.s b/llvm/test/MC/PowerPC/aix-weak-definition.s
new file mode 100644
index 0000000000000..6bab51b91c63d
--- /dev/null
+++ b/llvm/test/MC/PowerPC/aix-weak-definition.s
@@ -0,0 +1,13 @@
+## Check that weak_definition symbols are emitted as weak externals.
+# Note: On AIX, .weak_definition is mapped to .weak by LLVM's backend.
+# RUN: llvm-mc -triple powerpc-ibm-aix-xcoff %s -filetype=obj -o - | \
+# RUN:   llvm-objdump --syms - | FileCheck %s
+
+        .weak_definition foo  # LLVM IR WeakDefinition → .weak on AIX
+foo:
+        blr
+
+# CHECK:      SYMBOL TABLE:
+# CHECK-NEXT: 00000000      df *DEBUG*    00000000 .file
+# CHECK-NEXT: 00000000 l       .text    00000004
+# CHECK-NEXT: 00000000 w     F .text (csect: )  00000000 foo

@aabhinavg1 aabhinavg1 requested a review from MaskRay September 1, 2025 17:48
@tonykuttai
Copy link
Contributor

Adding @hubert-reinterpretcast & @daltenty.

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

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

Do you need .weak_definition? ELF doesn't support .weak_definition and we don't use MCSA_WeakDefinition for ELF. You can use MCSA_Weak for XCOFF

@folkertdev
Copy link
Contributor

folkertdev commented Oct 28, 2025

From the issue:

the .weak assembler directive is currently not supported by LLVM (although it is documented https://www.ibm.com/docs/en/ssw_aix_71/assembler/assembler_pdf.pdf).

In contrast, the .weak_definition directive is accepted by the assembly parser, but crashes LLVM later on.

Therefore it is currently impossible to define weak symbols using inline assembly.

So that might mean that actually .weak_definition should be rejected by the parser, and instead .weak should create weak externals?

At the very least .weak is documented so it should work. What .weak_definition does is undocumented, though having it do the same as .weak seems reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:PowerPC llvm:mc Machine (object) code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

powerpc64-ibm-aix: the .weak assembler directive is not supported

5 participants