Page MenuHomePhabricator

[AIX] Emit AvailableExternally Linkage on AIX
ClosedPublic

Authored by Xiangling_L on May 27 2020, 8:55 AM.

Details

Summary

There are two usages of AvailableExternally Linkage.

The first one is used for inlining of thunks under the Itanium ABI. However, this linkage set on thunks is a temporary presence and is never ended up in the final IR. So we don't need to take care of it in the backend.

The second one is where we have a const declaration with an initializer, we may be able to emit it as available_externally to expose its value to the optimizer. Since on AIX, our strategy is to not use -u to suppress any undefined symbols, we need to emit .extern for the symbol.

Diff Detail

Event Timeline

Xiangling_L created this revision.May 27 2020, 8:55 AM
DiggerLin added inline comments.May 27 2020, 12:07 PM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
442

I think we look AvailableExternallyLinkage as AvailableExternallyLinkage

you can change as

case GlobalValue::ExternalLinkage:
case GlobalValue::AvailableExternallyLinkage:
    if (MAI->hasDotExternDirective() && GV->isDeclarationForLinker()) {
}

and we do not need code

`
case GlobalValue::AvailableExternallyLinkage:
    if (TM.getTargetTriple().isOSBinFormatXCOFF()) {
   ....
}
`

here.

DiggerLin added inline comments.May 27 2020, 12:09 PM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
442

sorry it should be "I think we look AvailableExternallyLinkage as ExternalLinkage"

Xiangling_L marked 3 inline comments as done.May 27 2020, 1:54 PM
Xiangling_L added inline comments.
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
442

Thank you for your suggestion. I agree that it would give the query more semantic meaning. But since if a GV falls under the AvailableExternallyLinkage category, then GV->isDeclarationForLinker()is always true. So it seems we only need MAI->hasDotExternDirective() here?

DiggerLin added inline comments.May 27 2020, 2:20 PM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
442

but for ExternalLinkage, it need GV->isDeclarationForLinker(). otherwise it will emit .globl symbolName.

Xiangling_L marked 2 inline comments as done.May 27 2020, 7:11 PM
Xiangling_L added inline comments.
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
442

Sorry, I don't make it clear. But I don't think we should combine ExternalLinkage and AvailableExternallyLinkage. Because for other targets, we still want to fall throught to llvm_unreachable.

DiggerLin added inline comments.May 28 2020, 6:37 AM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
442

o, yes, you are correct, for other target, we still want to fa ll throught to llvm_unreachable. we maybe to change
if (TM.getTargetTriple().isOSBinFormatXCOFF()) to
if (MAI->hasDotExternDirective())

Xiangling_L marked 2 inline comments as done.

Change the query to give it more semantic meaning

jasonliu added inline comments.May 28 2020, 8:06 AM
llvm/test/CodeGen/PowerPC/aix-available-externally-linkage.ll
33

Test IR could be simplify to:

@_ZN1A1aE = available_externally constant i32 -1, align 4
@b = constant i32* @_ZN1A1aE, align 4
jasonliu added inline comments.May 28 2020, 8:58 AM
llvm/test/CodeGen/PowerPC/aix-available-externally-linkage.ll
3

https://reviews.llvm.org/D78929#inline-740137
suggested how to format the command if needs split.

Xiangling_L marked 4 inline comments as done.

Adjust the format of the testcase;
Simplify the testcase;

jasonliu accepted this revision.May 28 2020, 3:17 PM

LGTM with minor nit.

llvm/test/CodeGen/PowerPC/aix-available-externally-linkage.ll
16

nit: It's a bit weird to split between '-mtriple' and 'powerpc64-ibm-aix-xcoff'.

This revision is now accepted and ready to land.May 28 2020, 3:17 PM
llvm/test/CodeGen/PowerPC/aix-available-externally-linkage.ll
16

Also, the other cases have the triple before the processor level selection.

This revision was automatically updated to reflect the committed changes.