Page MenuHomePhabricator

[PowerPC] Provide MMA builtins for compatibility
ClosedPublic

Authored by saghir on Apr 14 2021, 7:54 AM.

Details

Summary

Vector pair intrinsics and builtins were renamed in
https://reviews.llvm.org/D91974 to replace the _mma_ prefix by _vsx_.
However, some projects used the _mma_ version, so this patch adds
these intrinsics to provide compatibility.

Fixes Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=50159

Diff Detail

Event Timeline

saghir created this revision.Apr 14 2021, 7:54 AM
saghir requested review of this revision.Apr 14 2021, 7:54 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 14 2021, 7:54 AM
amyk accepted this revision.Apr 14 2021, 7:05 PM
amyk added a subscriber: amyk.

LGTM.

clang/include/clang/Basic/BuiltinsPPC.def
696

Minor typo.

This revision is now accepted and ready to land.Apr 14 2021, 7:05 PM
saghir updated this revision to Diff 337765.Thu, Apr 15, 7:37 AM

Fixed spelling.

saghir retitled this revision from [PowerPC] Provide MMA builtins for compatability to [PowerPC] Provide MMA builtins for compatibility.Thu, Apr 15, 7:44 AM
saghir edited the summary of this revision. (Show Details)
nemanjai requested changes to this revision.Tue, Apr 27, 3:56 AM

There may be something I am overlooking here, but I really don't think we need to or want to change the back end. Just add the new builtins to the front end as aliases to the existing ones and generate the same code from the front end.

Also, we will want this backported to 12.0.1. Please open a bugzilla bug that we can mark as blocking the release and add a link to that PR in this review.

clang/lib/CodeGen/CGBuiltin.cpp
15366

The _mma_ version of the builtin is supposed to just be an alias for the _vsx_ version. There is no need to add a new intrinsic and produce it here. Just produce the same intrinsic for both builtins.

llvm/include/llvm/IR/IntrinsicsPowerPC.td
1139 ↗(On Diff #337765)

Please don't define these unless we absolutely need them.

If we don't add these, I think all the code in the back end is no longer needed.

This revision now requires changes to proceed.Tue, Apr 27, 3:56 AM
saghir updated this revision to Diff 342732.Tue, May 4, 7:32 AM

Addressed review comments to add _mma_ version of the built-ins as
aliases to the existing _vsx_ versions.

saghir edited the summary of this revision. (Show Details)Tue, May 4, 7:36 AM
nemanjai accepted this revision.Fri, May 7, 4:36 AM

LGTM. Thanks for fixing this.

This revision is now accepted and ready to land.Fri, May 7, 4:36 AM
This revision was landed with ongoing or failed builds.Fri, May 7, 7:10 AM
This revision was automatically updated to reflect the committed changes.