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

[Chris Bowler via Phabricator via llvm-commits]

cebowleratibm marked 3 inline comments as done.
cebowleratibm added a comment.

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



================
Comment at: llvm/test/CodeGen/PowerPC/aix-complex.ll:11
+
+ at gcd1 = external global { double, double }, align 8
+ at gcd2 = external global { double, double }, align 8
----------------
Xiangling_L wrote:
> 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.
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.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-complex.ll:19
+  %retval = alloca { double, double }, align 8
+  %gcd1.real = load double, double* getelementptr inbounds ({ double, double }, { double, double }* @gcd1, i32 0, i32 0), align 8
+  %gcd1.imag = load double, double* getelementptr inbounds ({ double, double }, { double, double }* @gcd1, i32 0, i32 1), align 8
----------------
Xiangling_L wrote:
> #19 - #24 can be removed.
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84069/new/

https://reviews.llvm.org/D84069