[NFC][PPC][AIX] Add test coverage for _Complex return values

[cebowleratibm]

Summary

This patch validates that llc has implemented the AIX ABI for _Complex float, _Complex double in 32 and 64 bit modes. I've also validated the correct handling of ppc_fp128 _Complex, though we don't have a front-end that is expected to emit such IR yet.

[ZarkoCA]

LGTM but I will leave it up for a bit so other reviewers have a chance to look at it.

[Xiangling_L]

llvm/test/CodeGen/PowerPC/aix-complex.ll

1 ↗

(On Diff #278886)

https://reviews.llvm.org/D78929#inline-740137
suggested how to format the command.

11 ↗

(On Diff #278886)

It seems all align xx in this test are unnecessary, so you can simplify the testcase by removing them.

Also attributes like noinline nounwind optnone can be removed.

19 ↗

(On Diff #278886)

#19 - #24 can be removed.

60 ↗

(On Diff #278886)

line #60 - #65 can be removed

102 ↗

(On Diff #278886)

#102 - #107 can be removed

[cebowleratibm]

I'll format the RUN steps as suggested but with respect to the other suggestions, I prefer to keep the test as-is.

llvm/test/CodeGen/PowerPC/aix-complex.ll

11 ↗

(On Diff #278886)

I generated the IR from C code, hence the align 8. I don't think it's strictly necessary but I'd like to keep the IR consistent with what we'd expect from a producer. I don't feel the alignment obfuscates the test.

Re attributes: I wanted optnone because optimization can elide the calling registers loads if it already knows the registers already hold the desired value. I wanted the expected register state right next to the bl/blr so I chose no-opt for this purpose.

19 ↗

(On Diff #278886)

If we're ok with returning an uninitialized value yes, and it probably would work as expected, however, backends reserve the right to discard use of uninitialized values. Theoretically, a backend could discard the return register initialization all together. Mitigated by optnone, however, I think it's best not to return an uninitialized value.

[cebowleratibm]

Formatted the RUN steps.

[cebowleratibm]

Modified the tests to work at opt. Replaced initialization of return values with literals to simplify the IR.

[cebowleratibm]

added some commentary notes for the review.

llvm/test/CodeGen/PowerPC/aix-complex.ll

25 ↗

(On Diff #279654)

Note this is lfs and not lfd. llc appears to be smart enough to use a 4 byte lfs to populate the register when possible.

91 ↗

(On Diff #279654)

The bit patterns were chosen deliberately to avoid redundancy otherwise llc will initialize the call registers with reg copies rather than lfd.

[Xiangling_L]

llvm/test/CodeGen/PowerPC/aix-complex.ll

36 ↗

(On Diff #279654)

Just a question, why we would like to branch to anchor here? Does it help us with testing _Complex?

[Xiangling_L]

llvm/test/CodeGen/PowerPC/aix-complex.ll

25 ↗

(On Diff #279654)

I guess it's because llc is not default to -O0, if you add -O0, you will get lfd back. And that leads me to think that you may want to add optimization level -O0 since you aimed at testing optnone scenario. Also you can test other optimization level if you are confident that the compiler is doing the right thing.

[cebowleratibm]

llvm/test/CodeGen/PowerPC/aix-complex.ll

36 ↗

(On Diff #279654)

Yes, it's so that there are no stray instructions that weave into the stfd's that are expected to follow the call to dblCmplxRetCallee.

; CHECK-NEXT: stfd 1, 0([[REG]])
; CHECK-NEXT: stfd 2, 8([[REG]])

is not ideal because we don't care what order the stfd's come out and

; CHECK-DAG: stfd 1, 0([[REG]])
; CHECK-DAG: stfd 2, 8([[REG]])

without the anchor would not require the stfd's to come immediately after the callee.

[cebowleratibm]

llvm/test/CodeGen/PowerPC/aix-complex.ll

25 ↗

(On Diff #279654)

The original intent of optnone was to make it easier to CHECK-NEXT against the expected register read/writes around the bl/blr. Instead I adapted the test in a way to run at opt. The intention of the test isn't to validate lfs/lfd between opt/noopt. I don't think the -O0 run is needed. If this test is run at -O0 it will fail, however, I think that's true of many LLVM tests.

[Xiangling_L]

LGTM. Thanks

[sfertile]

LGTM.

[dfukalov]

Hi @cebowleratibm, it seems test is located in wrong place: test/CodeGen/PowerPC instead of llvm/test/CodeGen/PowerPC.

[jsji]

In D84069#2183357, @dfukalov wrote:

Hi @cebowleratibm, it seems test is located in wrong place: test/CodeGen/PowerPC instead of llvm/test/CodeGen/PowerPC.

Fixed in https://reviews.llvm.org/rGdab8d6104bd7

test/CodeGen/PowerPC/aix-complex.ll

+; RUN: llc -verify-machineinstrs -mcpu=pwr4 -mattr=-altivec -mtriple powerpc-ibm-aix-xcoff < %s | \
+; RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
+; RUN: llc -verify-machineinstrs -mcpu=pwr4 -mattr=-altivec -mtriple powerpc64-ibm-aix-xcoff < %s | \
+; RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
+
+@gcd = external global { double, double }, align 8
+@gcf = external global { float, float }, align 4
+@gcfp128 = external global { ppc_fp128, ppc_fp128 }, align 16
+
+declare void @anchor(...)
+
+define dso_local { double, double } @dblCmplxRetCallee()  {
+entry:
+  %retval = alloca { double, double }, align 8
+  %retval.realp = getelementptr inbounds { double, double }, { double, double }* %retval, i32 0, i32 0
+  store double 1.000000e+00, double* %retval.realp, align 8
+  %retval.imagp = getelementptr inbounds { double, double }, { double, double }* %retval, i32 0, i32 1
+  store double 0.000000e+00, double* %retval.imagp, align 8
+  %0 = load { double, double }, { double, double }* %retval, align 8
+  ret { double, double } %0
+}
+
+; CHECK-LABEL: .dblCmplxRetCallee:
+
+; CHECK-DAG:   lfs 1,
+; CHECK-DAG:   lfs 2,
+; CHECK:       blr
+
+define dso_local void @dblCmplxRetCaller()  {
+entry:
+  %call = call { double, double } @dblCmplxRetCallee()
+  %0 = extractvalue { double, double } %call, 0
+  %1 = extractvalue { double, double } %call, 1
+  store double %0, double* getelementptr inbounds ({ double, double }, { double, double }* @gcd, i32 0, i32 0), align 8
+  store double %1, double* getelementptr inbounds ({ double, double }, { double, double }* @gcd, i32 0, i32 1), align 8
+  call void bitcast (void (...)* @anchor to void ()*)()
+  ret void
+}
+
+; CHECK-LABEL: .dblCmplxRetCaller:
+
+; CHECK:        bl .dblCmplxRetCallee
+; 32BIT-NEXT:   lwz [[REG:[0-9]+]], L..C{{[0-9]+}}(2)
+; 64BIT-NEXT:   ld [[REG:[0-9]+]], L..C{{[0-9]+}}(2)
+; CHECK-DAG:    stfd 1, 0([[REG]])
+; CHECK-DAG:    stfd 2, 8([[REG]])
+; CHECK-NEXT:   bl .anchor
+
+define dso_local { float, float } @fltCmplxRetCallee()  {
+entry:
+  %retval = alloca { float, float }, align 4
+  %retval.realp = getelementptr inbounds { float, float }, { float, float }* %retval, i32 0, i32 0
+  %retval.imagp = getelementptr inbounds { float, float }, { float, float }* %retval, i32 0, i32 1
+  store float 1.000000e+00, float* %retval.realp, align 4
+  store float 0.000000e+00, float* %retval.imagp, align 4
+  %0 = load { float, float }, { float, float }* %retval, align 4
+  ret { float, float } %0
+}
+
+; CHECK-LABEL: .fltCmplxRetCallee:
+
+; CHECK-DAG:   lfs 1,
+; CHECK-DAG:   lfs 2,
+; CHECK:       blr
+
+define dso_local void @fltCmplxRetCaller()  {
+entry:
+  %call = call { float, float } @fltCmplxRetCallee()
+  %0 = extractvalue { float, float } %call, 0
+  %1 = extractvalue { float, float } %call, 1
+  store float %0, float* getelementptr inbounds ({ float, float }, { float, float }* @gcf, i32 0, i32 0), align 4
+  store float %1, float* getelementptr inbounds ({ float, float }, { float, float }* @gcf, i32 0, i32 1), align 4
+  call void bitcast (void (...)* @anchor to void ()*)()
+  ret void
+}
+
+; CHECK-LABEL: .fltCmplxRetCaller:
+
+; CHECK:        bl .fltCmplxRetCallee
+; 32BIT-NEXT:   lwz [[REG:[0-9]+]], L..C{{[0-9]+}}(2)
+; 64BIT-NEXT:   ld [[REG:[0-9]+]], L..C{{[0-9]+}}(2)
+; CHECK-DAG:    stfs 1, 0([[REG]])
+; CHECK-DAG:    stfs 2, 4([[REG]])
+; CHECK-NEXT:   bl .anchor
+
+define dso_local { ppc_fp128, ppc_fp128 } @fp128CmplxRetCallee()  {
+entry:
+  %retval = alloca { ppc_fp128, ppc_fp128 }, align 16
+  %retval.realp = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* %retval, i32 0, i32 0
+  %retval.imagp = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* %retval, i32 0, i32 1
+  store ppc_fp128 0xM7ffeffffffffffffffffffffffffffff, ppc_fp128* %retval.realp, align 16
+  store ppc_fp128 0xM3ffefffffffffffffffffffffffffffe, ppc_fp128* %retval.imagp, align 16
+  %0 = load { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* %retval, align 16
+  ret { ppc_fp128, ppc_fp128 } %0
+}
+
+; CHECK-LABEL: .fp128CmplxRetCallee:
+
+; CHECK-DAG:  lfd 1,
+; CHECK-DAG:  lfd 2,
+; CHECK-DAG:  lfd 3,
+; CHECK-DAG:  lfd 4,
+; CHECK:      blr
+
+define dso_local void @fp128CmplxRetCaller()  {
+entry:
+  %call = call { ppc_fp128, ppc_fp128 } @fp128CmplxRetCallee()
+  %0 = extractvalue { ppc_fp128, ppc_fp128 } %call, 0
+  %1 = extractvalue { ppc_fp128, ppc_fp128 } %call, 1
+  store ppc_fp128 %0, ppc_fp128* getelementptr inbounds ({ ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* @gcfp128, i32 0, i32 0), align 16
+  store ppc_fp128 %1, ppc_fp128* getelementptr inbounds ({ ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* @gcfp128, i32 0, i32 1), align 16
+  call void bitcast (void (...)* @anchor to void ()*)()
+  ret void
+}
+
+; CHECK-LABEL: .fp128CmplxRetCaller:
+
+; CHECK:        bl .fp128CmplxRetCallee
+; 32BIT-NEXT:   lwz [[REG:[0-9]+]], L..C{{[0-9]+}}(2)
+; 64BIT-NEXT:   ld [[REG:[0-9]+]], L..C{{[0-9]+}}(2)
+; CHECK-DAG:    stfd 1, 0([[REG]])
+; CHECK-DAG:    stfd 2, 8([[REG]])
+; CHECK-DAG:    stfd 3, 16([[REG]])
+; CHECK-DAG:    stfd 4, 24([[REG]])
+; CHECK-NEXT:   bl .anchor