This is an archive of the discontinued LLVM Phabricator instance.

Differential D107138

[PowerPC] Implement cmplxl builtins
ClosedPublic

Authored by Conanap on Jul 30 2021, 1:28 AM.

Details

Reviewers
nemanjai
NeHuang
Group Reviewers
Restricted Project
Commits
rG9d4faa8ac3e7: [PowerPC] Implement cmplxl builtins
Summary

This patch implements the builtins for cmplxl by utilising
__builtin_complex. This builtin is implemented to match XL
functionality.

Diff Detail

Event Timeline

Conanap created this revision.Jul 30 2021, 1:28 AM
Herald added subscribers: shchenz, kbarton, nemanjai. · View Herald TranscriptJul 30 2021, 1:28 AM
Conanap requested review of this revision.Jul 30 2021, 1:28 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 30 2021, 1:28 AM
Herald added subscribers: llvm-commits, cfe-commits. · View Herald Transcript
Conanap added reviewers: Restricted Project, nemanjai.Jul 30 2021, 1:28 AM
Harbormaster completed remote builds in B117121: Diff 362992.Jul 30 2021, 2:21 AM
nemanjai requested changes to this revision.Jul 30 2021, 5:49 AM
nemanjai added inline comments.
clang/include/clang/Basic/BuiltinsPPC.def
146 ↗(On Diff #362992)

Please remove this. The preprocessor will replace this and the builtin call will never make it to the front end.

clang/lib/Basic/Targets/PPC.cpp
239

I don't see a compelling reason to have this. Users that want this functionality in new code (that doesn't already use __cmplxl) can simply use __builtin_complex.

clang/test/CodeGen/builtins-ppc-xlcompat-complex.c
45 ↗(On Diff #362992)

We really only need this test case and we should be able to just add it to one of the existing XL-compat clang test cases.

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-complex-32bit-only.ll
1 ↗(On Diff #362992)

I don't think we need the back end tests. No new IR is produced in this patch.

This revision now requires changes to proceed.Jul 30 2021, 5:49 AM
NeHuang added a subscriber: NeHuang.Jul 30 2021, 11:51 AM
NeHuang added inline comments.
clang/test/CodeGen/builtins-ppc-xlcompat-complex.c
1 ↗(On Diff #362992)

// REQUIRES: powerpc-registered-target

1 ↗(On Diff #362992)

Question: why do we need -O2 for this builtin?

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-complex-32bit-only.ll
1 ↗(On Diff #362992)

+1

amyk added a subscriber: amyk.Jul 30 2021, 1:30 PM
amyk added inline comments.
clang/test/CodeGen/builtins-ppc-xlcompat-complex.c
35 ↗(On Diff #362992)

I think it would probably be better not to hardcode the variable names in the tests.

Conanap updated this revision to Diff 364223.Aug 4 2021, 12:45 PM
Conanap marked 6 inline comments as done.

Removed backend tests, removed some uneeded definitions,
updated frontend test with regex for variable names.

Conanap marked an inline comment as done.Aug 4 2021, 12:45 PM
Conanap added inline comments.
clang/test/CodeGen/builtins-ppc-xlcompat-complex.c
1 ↗(On Diff #362992)

it's not required, but removes a lot of the extra load and stores that make the test cases longer unnecessarily. I can change it to O1 if preferred.

Harbormaster completed remote builds in B117985: Diff 364223.Aug 4 2021, 1:28 PM
NeHuang added inline comments.Aug 6 2021, 7:32 AM
clang/test/CodeGen/builtins-ppc-xlcompat-complex.c
45 ↗(On Diff #362992)

As suggest above, can we add the test case __cmplxl to one of the existing clang test file builtins-ppc-xlcompat-cmplx.c? You can auto generate the CHECKS using utils/update_cc_test_checks.py to avoid hardcoding the variable names.

nemanjai added inline comments.Aug 7 2021, 9:45 AM
clang/test/CodeGen/builtins-ppc-xlcompat-complex.c
1 ↗(On Diff #362992)

I prefer that front end tests should test what the front end does. The front end does not perform optimizations, the optimizer does. So the front end tests should not include optimization options (even though I do realize that adding -O<N> reduces the size of the produced code).
There is no need to add unnecessary churn in front end tests if the optimizer changes.

Conanap updated this revision to Diff 365289.Aug 9 2021, 2:34 PM
Conanap marked 3 inline comments as done.

Merged the test case into the existing cmplx test

Harbormaster completed remote builds in B118756: Diff 365289.Aug 9 2021, 3:37 PM
Conanap updated this revision to Diff 365596.Aug 10 2021, 1:06 PM

Removed unintended change

Harbormaster completed remote builds in B118966: Diff 365596.Aug 10 2021, 2:35 PM
nemanjai accepted this revision.Aug 10 2021, 2:44 PM

LGTM.

This revision is now accepted and ready to land.Aug 10 2021, 2:44 PM
NeHuang accepted this revision.Aug 11 2021, 5:56 AM

LGTM

This revision was landed with ongoing or failed builds.Aug 19 2021, 7:39 PM
Closed by commit rG9d4faa8ac3e7: [PowerPC] Implement cmplxl builtins (authored by Conanap). · Explain Why
This revision was automatically updated to reflect the committed changes.
Conanap added a commit: rG9d4faa8ac3e7: [PowerPC] Implement cmplxl builtins.

Revision Contents

PathSize
clang/
lib/
Basic/
Targets/
1 line
test/
CodeGen/
112 lines

Diff 367695

clang/lib/Basic/Targets/PPC.cpp

Show First 20 Lines • Show All 230 Lines • ▼ Show 20 Linesstatic void defineXLCompatMacros(MacroBuilder &Builder) {
Builder.defineMacro("__friz", "__builtin_ppc_friz"); Builder.defineMacro("__friz", "__builtin_ppc_friz");
Builder.defineMacro("__frizs", "__builtin_ppc_frizs"); Builder.defineMacro("__frizs", "__builtin_ppc_frizs");
Builder.defineMacro("__fsel", "__builtin_ppc_fsel"); Builder.defineMacro("__fsel", "__builtin_ppc_fsel");
Builder.defineMacro("__fsels", "__builtin_ppc_fsels"); Builder.defineMacro("__fsels", "__builtin_ppc_fsels");
Builder.defineMacro("__frsqrte", "__builtin_ppc_frsqrte"); Builder.defineMacro("__frsqrte", "__builtin_ppc_frsqrte");
Builder.defineMacro("__frsqrtes", "__builtin_ppc_frsqrtes"); Builder.defineMacro("__frsqrtes", "__builtin_ppc_frsqrtes");
Builder.defineMacro("__fsqrt", "__builtin_ppc_fsqrt"); Builder.defineMacro("__fsqrt", "__builtin_ppc_fsqrt");
Builder.defineMacro("__fsqrts", "__builtin_ppc_fsqrts"); Builder.defineMacro("__fsqrts", "__builtin_ppc_fsqrts");
Builder.defineMacro("__addex", "__builtin_ppc_addex"); Builder.defineMacro("__addex", "__builtin_ppc_addex");
nemanjaiUnsubmitted
Done
ReplyInline Actions

I don't see a compelling reason to have this. Users that want this functionality in new code (that doesn't already use __cmplxl) can simply use __builtin_complex.

nemanjai: I don't see a compelling reason to have this. Users that want this functionality in new code…
Builder.defineMacro("__cmplxl", "__builtin_complex");
} }
/// PPCTargetInfo::getTargetDefines - Return a set of the PowerPC-specific /// PPCTargetInfo::getTargetDefines - Return a set of the PowerPC-specific
/// #defines that are not tied to a specific subtarget. /// #defines that are not tied to a specific subtarget.
void PPCTargetInfo::getTargetDefines(const LangOptions &Opts, void PPCTargetInfo::getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const { MacroBuilder &Builder) const {
defineXLCompatMacros(Builder); defineXLCompatMacros(Builder);
▲ Show 20 Lines • Show All 535 Lines • Show Last 20 Lines

clang/test/CodeGen/builtins-ppc-xlcompat-cmplx.c

Show First 20 Lines • Show All 220 Lines • ▼ Show 20 Lines
// 32BITAIX-NEXT: store float [[TMP0]], float* [[RETVAL_REALP]], align 4 // 32BITAIX-NEXT: store float [[TMP0]], float* [[RETVAL_REALP]], align 4
// 32BITAIX-NEXT: store float [[TMP1]], float* [[RETVAL_IMAGP]], align 4 // 32BITAIX-NEXT: store float [[TMP1]], float* [[RETVAL_IMAGP]], align 4
// 32BITAIX-NEXT: [[TMP2:%.*]] = load { float, float }, { float, float }* [[RETVAL]], align 4 // 32BITAIX-NEXT: [[TMP2:%.*]] = load { float, float }, { float, float }* [[RETVAL]], align 4
// 32BITAIX-NEXT: ret { float, float } [[TMP2]] // 32BITAIX-NEXT: ret { float, float } [[TMP2]]
// //
float _Complex testcmplxf(float real, float imag) { float _Complex testcmplxf(float real, float imag) {
return __cmplxf(real, imag); return __cmplxf(real, imag);
} }
// 64BIT-LABEL: @test_xl_cmplxl(
// 64BIT-NEXT: entry:
// 64BIT-NEXT: [[RETVAL:%.*]] = alloca { ppc_fp128, ppc_fp128 }, align 16
// 64BIT-NEXT: [[LDA_ADDR:%.*]] = alloca ppc_fp128, align 16
// 64BIT-NEXT: [[LDB_ADDR:%.*]] = alloca ppc_fp128, align 16
// 64BIT-NEXT: store ppc_fp128 [[LDA:%.*]], ppc_fp128* [[LDA_ADDR]], align 16
// 64BIT-NEXT: store ppc_fp128 [[LDB:%.*]], ppc_fp128* [[LDB_ADDR]], align 16
// 64BIT-NEXT: [[TMP0:%.*]] = load ppc_fp128, ppc_fp128* [[LDA_ADDR]], align 16
// 64BIT-NEXT: [[TMP1:%.*]] = load ppc_fp128, ppc_fp128* [[LDB_ADDR]], align 16
// 64BIT-NEXT: [[RETVAL_REALP:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[RETVAL]], i32 0, i32 0
// 64BIT-NEXT: [[RETVAL_IMAGP:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[RETVAL]], i32 0, i32 1
// 64BIT-NEXT: store ppc_fp128 [[TMP0]], ppc_fp128* [[RETVAL_REALP]], align 16
// 64BIT-NEXT: store ppc_fp128 [[TMP1]], ppc_fp128* [[RETVAL_IMAGP]], align 16
// 64BIT-NEXT: [[TMP2:%.*]] = load { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[RETVAL]], align 16
// 64BIT-NEXT: ret { ppc_fp128, ppc_fp128 } [[TMP2]]
//
// 64BITLE-LABEL: @test_xl_cmplxl(
// 64BITLE-NEXT: entry:
// 64BITLE-NEXT: [[RETVAL:%.*]] = alloca { ppc_fp128, ppc_fp128 }, align 16
// 64BITLE-NEXT: [[LDA_ADDR:%.*]] = alloca ppc_fp128, align 16
// 64BITLE-NEXT: [[LDB_ADDR:%.*]] = alloca ppc_fp128, align 16
// 64BITLE-NEXT: store ppc_fp128 [[LDA:%.*]], ppc_fp128* [[LDA_ADDR]], align 16
// 64BITLE-NEXT: store ppc_fp128 [[LDB:%.*]], ppc_fp128* [[LDB_ADDR]], align 16
// 64BITLE-NEXT: [[TMP0:%.*]] = load ppc_fp128, ppc_fp128* [[LDA_ADDR]], align 16
// 64BITLE-NEXT: [[TMP1:%.*]] = load ppc_fp128, ppc_fp128* [[LDB_ADDR]], align 16
// 64BITLE-NEXT: [[RETVAL_REALP:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[RETVAL]], i32 0, i32 0
// 64BITLE-NEXT: [[RETVAL_IMAGP:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[RETVAL]], i32 0, i32 1
// 64BITLE-NEXT: store ppc_fp128 [[TMP0]], ppc_fp128* [[RETVAL_REALP]], align 16
// 64BITLE-NEXT: store ppc_fp128 [[TMP1]], ppc_fp128* [[RETVAL_IMAGP]], align 16
// 64BITLE-NEXT: [[TMP2:%.*]] = load { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[RETVAL]], align 16
// 64BITLE-NEXT: ret { ppc_fp128, ppc_fp128 } [[TMP2]]
//
// 64BITAIX-LABEL: @test_xl_cmplxl(
// 64BITAIX-NEXT: entry:
// 64BITAIX-NEXT: [[RETVAL:%.*]] = alloca { double, double }, align 4
// 64BITAIX-NEXT: [[LDA_ADDR:%.*]] = alloca double, align 8
// 64BITAIX-NEXT: [[LDB_ADDR:%.*]] = alloca double, align 8
// 64BITAIX-NEXT: store double [[LDA:%.*]], double* [[LDA_ADDR]], align 8
// 64BITAIX-NEXT: store double [[LDB:%.*]], double* [[LDB_ADDR]], align 8
// 64BITAIX-NEXT: [[TMP0:%.*]] = load double, double* [[LDA_ADDR]], align 8
// 64BITAIX-NEXT: [[TMP1:%.*]] = load double, double* [[LDB_ADDR]], align 8
// 64BITAIX-NEXT: [[RETVAL_REALP:%.*]] = getelementptr inbounds { double, double }, { double, double }* [[RETVAL]], i32 0, i32 0
// 64BITAIX-NEXT: [[RETVAL_IMAGP:%.*]] = getelementptr inbounds { double, double }, { double, double }* [[RETVAL]], i32 0, i32 1
// 64BITAIX-NEXT: store double [[TMP0]], double* [[RETVAL_REALP]], align 4
// 64BITAIX-NEXT: store double [[TMP1]], double* [[RETVAL_IMAGP]], align 4
// 64BITAIX-NEXT: [[TMP2:%.*]] = load { double, double }, { double, double }* [[RETVAL]], align 4
// 64BITAIX-NEXT: ret { double, double } [[TMP2]]
//
// 32BIT-LABEL: @test_xl_cmplxl(
// 32BIT-NEXT: entry:
// 32BIT-NEXT: [[LDA_ADDR:%.*]] = alloca ppc_fp128, align 16
// 32BIT-NEXT: [[LDB_ADDR:%.*]] = alloca ppc_fp128, align 16
// 32BIT-NEXT: store ppc_fp128 [[LDA:%.*]], ppc_fp128* [[LDA_ADDR]], align 16
// 32BIT-NEXT: store ppc_fp128 [[LDB:%.*]], ppc_fp128* [[LDB_ADDR]], align 16
// 32BIT-NEXT: [[TMP0:%.*]] = load ppc_fp128, ppc_fp128* [[LDA_ADDR]], align 16
// 32BIT-NEXT: [[TMP1:%.*]] = load ppc_fp128, ppc_fp128* [[LDB_ADDR]], align 16
// 32BIT-NEXT: [[AGG_RESULT_REALP:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[AGG_RESULT:%.*]], i32 0, i32 0
// 32BIT-NEXT: [[AGG_RESULT_IMAGP:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[AGG_RESULT]], i32 0, i32 1
// 32BIT-NEXT: store ppc_fp128 [[TMP0]], ppc_fp128* [[AGG_RESULT_REALP]], align 16
// 32BIT-NEXT: store ppc_fp128 [[TMP1]], ppc_fp128* [[AGG_RESULT_IMAGP]], align 16
// 32BIT-NEXT: [[AGG_RESULT_REALP1:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[AGG_RESULT]], i32 0, i32 0
// 32BIT-NEXT: [[AGG_RESULT_REAL:%.*]] = load ppc_fp128, ppc_fp128* [[AGG_RESULT_REALP1]], align 16
// 32BIT-NEXT: [[AGG_RESULT_IMAGP2:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[AGG_RESULT]], i32 0, i32 1
// 32BIT-NEXT: [[AGG_RESULT_IMAG:%.*]] = load ppc_fp128, ppc_fp128* [[AGG_RESULT_IMAGP2]], align 16
// 32BIT-NEXT: [[AGG_RESULT_REALP3:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[AGG_RESULT]], i32 0, i32 0
// 32BIT-NEXT: [[AGG_RESULT_IMAGP4:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[AGG_RESULT]], i32 0, i32 1
// 32BIT-NEXT: store ppc_fp128 [[AGG_RESULT_REAL]], ppc_fp128* [[AGG_RESULT_REALP3]], align 16
// 32BIT-NEXT: store ppc_fp128 [[AGG_RESULT_IMAG]], ppc_fp128* [[AGG_RESULT_IMAGP4]], align 16
// 32BIT-NEXT: ret void
//
// 32BITLE-LABEL: @test_xl_cmplxl(
// 32BITLE-NEXT: entry:
// 32BITLE-NEXT: [[LDA_ADDR:%.*]] = alloca ppc_fp128, align 16
// 32BITLE-NEXT: [[LDB_ADDR:%.*]] = alloca ppc_fp128, align 16
// 32BITLE-NEXT: store ppc_fp128 [[LDA:%.*]], ppc_fp128* [[LDA_ADDR]], align 16
// 32BITLE-NEXT: store ppc_fp128 [[LDB:%.*]], ppc_fp128* [[LDB_ADDR]], align 16
// 32BITLE-NEXT: [[TMP0:%.*]] = load ppc_fp128, ppc_fp128* [[LDA_ADDR]], align 16
// 32BITLE-NEXT: [[TMP1:%.*]] = load ppc_fp128, ppc_fp128* [[LDB_ADDR]], align 16
// 32BITLE-NEXT: [[AGG_RESULT_REALP:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[AGG_RESULT:%.*]], i32 0, i32 0
// 32BITLE-NEXT: [[AGG_RESULT_IMAGP:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[AGG_RESULT]], i32 0, i32 1
// 32BITLE-NEXT: store ppc_fp128 [[TMP0]], ppc_fp128* [[AGG_RESULT_REALP]], align 16
// 32BITLE-NEXT: store ppc_fp128 [[TMP1]], ppc_fp128* [[AGG_RESULT_IMAGP]], align 16
// 32BITLE-NEXT: [[AGG_RESULT_REALP1:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[AGG_RESULT]], i32 0, i32 0
// 32BITLE-NEXT: [[AGG_RESULT_REAL:%.*]] = load ppc_fp128, ppc_fp128* [[AGG_RESULT_REALP1]], align 16
// 32BITLE-NEXT: [[AGG_RESULT_IMAGP2:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[AGG_RESULT]], i32 0, i32 1
// 32BITLE-NEXT: [[AGG_RESULT_IMAG:%.*]] = load ppc_fp128, ppc_fp128* [[AGG_RESULT_IMAGP2]], align 16
// 32BITLE-NEXT: [[AGG_RESULT_REALP3:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[AGG_RESULT]], i32 0, i32 0
// 32BITLE-NEXT: [[AGG_RESULT_IMAGP4:%.*]] = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* [[AGG_RESULT]], i32 0, i32 1
// 32BITLE-NEXT: store ppc_fp128 [[AGG_RESULT_REAL]], ppc_fp128* [[AGG_RESULT_REALP3]], align 16
// 32BITLE-NEXT: store ppc_fp128 [[AGG_RESULT_IMAG]], ppc_fp128* [[AGG_RESULT_IMAGP4]], align 16
// 32BITLE-NEXT: ret void
//
// 32BITAIX-LABEL: @test_xl_cmplxl(
// 32BITAIX-NEXT: entry:
// 32BITAIX-NEXT: [[RETVAL:%.*]] = alloca { double, double }, align 4
// 32BITAIX-NEXT: [[LDA_ADDR:%.*]] = alloca double, align 8
// 32BITAIX-NEXT: [[LDB_ADDR:%.*]] = alloca double, align 8
// 32BITAIX-NEXT: store double [[LDA:%.*]], double* [[LDA_ADDR]], align 8
// 32BITAIX-NEXT: store double [[LDB:%.*]], double* [[LDB_ADDR]], align 8
// 32BITAIX-NEXT: [[TMP0:%.*]] = load double, double* [[LDA_ADDR]], align 8
// 32BITAIX-NEXT: [[TMP1:%.*]] = load double, double* [[LDB_ADDR]], align 8
// 32BITAIX-NEXT: [[RETVAL_REALP:%.*]] = getelementptr inbounds { double, double }, { double, double }* [[RETVAL]], i32 0, i32 0
// 32BITAIX-NEXT: [[RETVAL_IMAGP:%.*]] = getelementptr inbounds { double, double }, { double, double }* [[RETVAL]], i32 0, i32 1
// 32BITAIX-NEXT: store double [[TMP0]], double* [[RETVAL_REALP]], align 4
// 32BITAIX-NEXT: store double [[TMP1]], double* [[RETVAL_IMAGP]], align 4
// 32BITAIX-NEXT: [[TMP2:%.*]] = load { double, double }, { double, double }* [[RETVAL]], align 4
// 32BITAIX-NEXT: ret { double, double } [[TMP2]]
//
long double _Complex test_xl_cmplxl(long double lda, long double ldb) {
return __cmplxl(lda, ldb);
}