This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Fix lharx and lbarx builtin signatures
ClosedPublic

Authored by Conanap on Sep 22 2021, 11:09 AM.

Details

Summary

The signatures for the PowerPC builtins lharx and
lbarx are incorrect, and causes issues when in a function
that requiers the return of the builtin to be promoted.
This patch fixes these signatures.

Diff Detail

Event Timeline

Conanap created this revision.Sep 22 2021, 11:09 AM
Conanap requested review of this revision.Sep 22 2021, 11:09 AM

The description says it causes issues but there is no test case. Please add the test case that causes issues.

Conanap updated this revision to Diff 374556.Sep 23 2021, 8:11 AM

Added test cases

NeHuang added inline comments.
clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
27 ↗(On Diff #374556)

Do you also need to update the input argument type here as well to match the changes in BuiltinsPPC.def?

Conanap updated this revision to Diff 374908.Sep 24 2021, 10:56 AM

Fixed an old test case

nemanjai accepted this revision.Sep 30 2021, 4:11 AM

LGTM. Thanks.

This revision is now accepted and ready to land.Sep 30 2021, 4:11 AM
amyk added a subscriber: amyk.Sep 30 2021, 6:31 AM

Additional nit regarding the description and comment:

The signatures for the PowerPC builtins lharx and
lbarx are incorrect, and causes issues when in a function
that requiers the return of the builtin to be promoted.
This patch fixes these signatures.

Updated to:

The signatures for the PowerPC builtins lharx and
lbarx are incorrect, and causes issues when used in a function
that requires the return of the builtin to be promoted.
This patch fixes these signatures.

clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
50 ↗(On Diff #374908)

nit: Capitalize the sentence and add a period.

This revision was landed with ongoing or failed builds.Sep 30 2021, 8:36 PM
This revision was automatically updated to reflect the committed changes.