This is an archive of the discontinued LLVM Phabricator instance.

Revert "[AIX] Avoid structor alias; die before bad alias codegen"
ClosedPublic

Authored by Jake-Egan on May 18 2021, 1:59 PM.

Details

Summary

Avoiding structor alias is no longer needed because AIX now has an alias implementation here: https://reviews.llvm.org/D83252.

This reverts commit b116ded57da3530e661f871f4191c59cd9e091cd.

Diff Detail

Event Timeline

Jake-Egan created this revision.May 18 2021, 1:59 PM
Jake-Egan requested review of this revision.May 18 2021, 1:59 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 18 2021, 1:59 PM
Jake-Egan edited the summary of this revision. (Show Details)May 18 2021, 2:01 PM
jsji added a reviewer: Restricted Project.May 18 2021, 2:13 PM
jasonliu added inline comments.May 18 2021, 2:28 PM
clang/lib/Driver/ToolChains/Clang.cpp
4976–4977

Format as clang-format suggest. You could run clang-format or git-clang-format just for your commits by the way.

clang/test/Driver/aix-constructor-alias.c
3

Instead of removing this file, we want to check if -mconstructor-aliases is passed in for AIX.
Check if there are other existing files doing it for other target, then you just need to add the check for AIX target in other files.

llvm/test/CodeGen/PowerPC/aix-alias.ll
4 ↗(On Diff #346270)

I don't think we want to remove the testing of our alias implementation.

Might be a dumb question, but I see this is a revert to rGb116ded, do we also want to revert the changes added to llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp?

clang/lib/Driver/ToolChains/Clang.cpp
4976–4977

FWIW, if clang-format is installed on your host (you can do this via homebrew on mac IIRC), arc would use it as the linter when you do arc lint or arc diff.

Might be a dumb question, but I see this is a revert to rGb116ded, do we also want to revert the changes added to llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp?

The changes to that file seems to be already reverted by: https://reviews.llvm.org/D79127

Jake-Egan updated this revision to Diff 346560.May 19 2021, 2:02 PM

Added the changes jasonliu asked for.

jasonliu accepted this revision.May 19 2021, 2:11 PM

LGTM.

This revision is now accepted and ready to land.May 19 2021, 2:11 PM
jsji added a subscriber: jsji.EditedMay 19 2021, 2:19 PM

because AIX now has an alias implementation.

Can we please include the patches or commits that implement the alias in description?

Good idea, fyi, this is the patch: https://reviews.llvm.org/D83252

Jake-Egan edited the summary of this revision. (Show Details)May 19 2021, 2:36 PM
jsji added a comment.May 19 2021, 2:51 PM

Good idea, fyi, this is the patch: https://reviews.llvm.org/D83252

Thanks.

This revision was landed with ongoing or failed builds.May 25 2021, 12:08 PM
This revision was automatically updated to reflect the committed changes.