[PowerPC] Alignment of toc-data symbol should not be increased during optimizations

[bzEq]

Currently, the alignment of toc-data symbol might be changed during instcombine

IC: Visiting:   %global = alloca %struct.widget, align 8                                                                                         
Found alloca equal to global:   %global = alloca %struct.widget, align 8                                                                         
  memcpy =   call void @llvm.memcpy.p0.p0.i64(ptr nonnull align 1 %global, ptr align 1 @global, i64 3, i1 false)

The alloca is created with PrefAlign which is 8 and after IC, the alignment of @global is enforced into 8, same as the alloca. This is not expected, since toc-data symbol has the same alignment as toc entry and should not be increased during optimizations.

[llvmbot]

@llvm/pr-subscribers-backend-powerpc

@llvm/pr-subscribers-llvm-ir

Author: Kai Luo (bzEq)

Changes

Currently, the alignment of toc-data symbol might be changed during instcombine

IC: Visiting:   %global = alloca %struct.widget, align 8                                                                                         
Found alloca equal to global:   %global = alloca %struct.widget, align 8                                                                         
  memcpy =   call void @<!-- -->llvm.memcpy.p0.p0.i64(ptr nonnull align 1 %global, ptr align 1 @<!-- -->global, i64 3, i1 false)

The alloca is created with PrefAlign which is 8 and after IC, the alignment of @<!-- -->global is enforced into 8, same as the alloca. This is not expected, since toc-data symbol has the same alignment as toc entry and should not be increased during optimizations.

---

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

2 Files Affected:

• (modified) llvm/lib/IR/Globals.cpp (+8)
• (modified) llvm/test/CodeGen/PowerPC/tocdata-firm-alignment.ll (+1-1)

diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 6f071847bb58..2943a2aa0cf6 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -335,6 +335,14 @@ bool GlobalObject::canIncreaseAlignment() const {
   if (isELF && !isDSOLocal())
     return false;
 
+  bool isXCOFF =
+      (!Parent || Triple(Parent->getTargetTriple()).isOSBinFormatXCOFF());
+  if (isXCOFF)
+    if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(this))
+      // GV with toc-data attribute is put in the region same as toc entry.
+      // Its alignment should be the same as toc entry.
+      return !GV->hasAttribute("toc-data");
+
   return true;
 }
 
diff --git a/llvm/test/CodeGen/PowerPC/tocdata-firm-alignment.ll b/llvm/test/CodeGen/PowerPC/tocdata-firm-alignment.ll
index c982713d4f8d..4ecec36bc977 100644
--- a/llvm/test/CodeGen/PowerPC/tocdata-firm-alignment.ll
+++ b/llvm/test/CodeGen/PowerPC/tocdata-firm-alignment.ll
@@ -5,7 +5,7 @@ target triple = "powerpc-ibm-aix7.2.0.0"
 
 %struct.widget = type { i8, i8, i8 }
 
-; CHECK: @global = {{.*}}constant %struct.widget { i8 4, i8 0, i8 0 }, align 8 #0
+; CHECK: @global = {{.*}}constant %struct.widget { i8 4, i8 0, i8 0 }, align 4 #0
 @global = constant %struct.widget { i8 4, i8 0, i8 0 }, align 4 #0
 
 define void @baz() #1 {

[mandlebug]

Would it be better suited to place the test in Transforms/InferAlignment/ instead?

[bzEq]

That looks like a place for InferAlignmentPass.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[MaskRay]

This is not expected, since toc-data symbol has the same alignment as toc entry and should not be increased during optimizations.

Why does AIX have this requirement? ppc64 ELFv2 ABI can increase the alignment without any issue.

[efriedma-quic]

From the perspective of the IR, this is the correct way to implement this restriction. (toc-data should be documented in LangRef, though.) I won't try to dive into the XCOFF side of this.

[bzEq]

Why does AIX have this requirement? ppc64 ELFv2 ABI can increase the alignment without any issue.

I tend to believe it's implementation's limitation, see

llvm-project/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

Line 2727 in f2441b0

assert(

.

[chenzheng1030]

Why does AIX have this requirement? ppc64 ELFv2 ABI can increase the alignment without any issue.

IIUC, AIX has limited TOC region especially for small code model and TOC region is created by linker(i.e., compiler can not fully control the layout of the TOC entries), so to avoid wasting of TOC entries, guarding alignment of the TOC data symbols be same with normal toc entries is necessary.

int a;
int foo()
{
  return a;
}

int b;
int main() {
  return b + foo();
}

If a is a toc-data symbol and we change its alignment to 16 in the assembly ( 2^16 is the size of the TOC region for small code model), compiling the modified assembly will make AIX linker emit TOC overflow error.

[MaskRay]

Why does AIX have this requirement? ppc64 ELFv2 ABI can increase the alignment without any issue.

IIUC, AIX has limited TOC region especially for small code model and TOC region is created by linker(i.e., compiler can not fully control the layout of the TOC entries), so to avoid wasting of TOC entries, guarding alignment of the TOC data symbols be same with normal toc entries is necessary.

int a;
int foo()
{
  return a;
}

int b;
int main() {
  return b + foo();
}

If a is a toc-data symbol and we change its alignment to 16 in the assembly ( 2^16 is the size of the TOC region for small code model), compiling the modified assembly will make AIX linker emit TOC overflow error.

Thanks for the example. I have also tried

clang -O2 -c a.c --target=powerpc64 -mabi=elfv{1,2} -mcmodel={small,medium}

AIX has the limited R_TOC relocation, similar to R_PPC64_TOC16_DS emitted by ELFv1 ELFv2 with -mcmodel=small.
I think nearly nobody uses -mcmodel=small for ELF, so having this exception only for AIX makes sense.

[MaskRay]

This comment can be lifted before isXCOFF. It seems not correct.

The alignment is not increased to mitigate TOC size overflow.

[bzEq]

toc-data should be documented in LangRef, though

Posted #94828.

[bzEq]

I'll leave doc change also in the same PR.

[bzEq]

Ping @efriedma-quic , could you please review the doc change?

[efriedma-quic]

New documentation looks fine. One minor suggestion for the code; otherwise LGTM