This is an archive of the discontinued LLVM Phabricator instance.

Paths

Table of Contentst

-
clang/include/clang/Basic/
-
include/
-
clang/
-
Basic/
-
BuiltinsPPC.def

Differential D110273

[PowerPC] Fix lharx and lbarx builtin signatures
ClosedPublic

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

Download Raw Diff

Details

Reviewers

nemanjai
stefanp
saghir

Group Reviewers

Restricted Project

Commits

rG29bb877499ce: [PowerPC] Fix lharx and lbarx builtin signatures

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

Unit TestsFailed

	Time	Test
	70 ms	x64 debian > ORC-x86_64-linux.TestCases/Linux/x86-64::trivial-cxa-atexit.S
	80 ms	x64 debian > ORC-x86_64-linux.TestCases/Linux/x86-64::trivial-static-initializer.S
	70 ms	x64 debian > ORC-x86_64-linux.TestCases/Linux/x86-64::trivial-tls.S

Event Timeline

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

Herald added subscribers: steven.zhang, kbarton. · View Herald TranscriptSep 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.

Harbormaster completed remote builds in B125175: Diff 374300.Sep 22 2021, 11:37 AM

Added test cases

Harbormaster completed remote builds in B125355: Diff 374556.Sep 23 2021, 8:45 AM

NeHuang added a subscriber: NeHuang.Sep 24 2021, 9:01 AM

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`?

Fixed an old test case

Harbormaster completed remote builds in B125613: Diff 374908.Sep 24 2021, 11:19 AM

LGTM. Thanks.

This revision is now accepted and ready to land.Sep 30 2021, 4:11 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

Closed by commit rG29bb877499ce: [PowerPC] Fix lharx and lbarx builtin signatures (authored by Conanap). · Explain Why

This revision was automatically updated to reflect the committed changes.

Conanap added a commit: rG29bb877499ce: [PowerPC] Fix lharx and lbarx builtin signatures.

Revision Contents

Path

Size

clang/

include/

clang/

Basic/

BuiltinsPPC.def

4 lines

Diff 374300

clang/include/clang/Basic/BuiltinsPPC.def

	Show First 20 Lines • Show All 68 Lines • ▼ Show 20 Lines
	BUILTIN(__builtin_ppc_fetch_and_and, "UiUiD*Ui", "")			BUILTIN(__builtin_ppc_fetch_and_and, "UiUiD*Ui", "")
	BUILTIN(__builtin_ppc_fetch_and_andlp, "ULiULiD*ULi", "")			BUILTIN(__builtin_ppc_fetch_and_andlp, "ULiULiD*ULi", "")
	BUILTIN(__builtin_ppc_fetch_and_or, "UiUiD*Ui", "")			BUILTIN(__builtin_ppc_fetch_and_or, "UiUiD*Ui", "")
	BUILTIN(__builtin_ppc_fetch_and_orlp, "ULiULiD*ULi", "")			BUILTIN(__builtin_ppc_fetch_and_orlp, "ULiULiD*ULi", "")
	BUILTIN(__builtin_ppc_fetch_and_swap, "UiUiD*Ui", "")			BUILTIN(__builtin_ppc_fetch_and_swap, "UiUiD*Ui", "")
	BUILTIN(__builtin_ppc_fetch_and_swaplp, "ULiULiD*ULi", "")			BUILTIN(__builtin_ppc_fetch_and_swaplp, "ULiULiD*ULi", "")
	BUILTIN(__builtin_ppc_ldarx, "LiLiD*", "")			BUILTIN(__builtin_ppc_ldarx, "LiLiD*", "")
	BUILTIN(__builtin_ppc_lwarx, "iiD*", "")			BUILTIN(__builtin_ppc_lwarx, "iiD*", "")
	BUILTIN(__builtin_ppc_lharx, "isD*", "")			BUILTIN(__builtin_ppc_lharx, "ssD*", "")
	BUILTIN(__builtin_ppc_lbarx, "UiUcD*", "")			BUILTIN(__builtin_ppc_lbarx, "ccD*", "")
	BUILTIN(__builtin_ppc_stdcx, "iLiD*Li", "")			BUILTIN(__builtin_ppc_stdcx, "iLiD*Li", "")
	BUILTIN(__builtin_ppc_stwcx, "iiD*i", "")			BUILTIN(__builtin_ppc_stwcx, "iiD*i", "")
	BUILTIN(__builtin_ppc_sthcx, "isD*s", "")			BUILTIN(__builtin_ppc_sthcx, "isD*s", "")
	BUILTIN(__builtin_ppc_stbcx, "icD*i", "")			BUILTIN(__builtin_ppc_stbcx, "icD*i", "")
	BUILTIN(__builtin_ppc_tdw, "vLLiLLiIUi", "")			BUILTIN(__builtin_ppc_tdw, "vLLiLLiIUi", "")
	BUILTIN(__builtin_ppc_tw, "viiIUi", "")			BUILTIN(__builtin_ppc_tw, "viiIUi", "")
	BUILTIN(__builtin_ppc_trap, "vi", "")			BUILTIN(__builtin_ppc_trap, "vi", "")
	BUILTIN(__builtin_ppc_trapd, "vLi", "")			BUILTIN(__builtin_ppc_trapd, "vLi", "")
	▲ Show 20 Lines • Show All 813 Lines • Show Last 20 Lines