[PATCH] D71013: [AIX] Allow vararg calls when all arguments reside in registers.

[Chris Bowler via Phabricator via llvm-commits]

cebowleratibm marked 8 inline comments as done.
cebowleratibm added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:697
+  %1 = load double, double* @d1, align 8
+  call void (i32, ...) @test_vararg(i32 42, double %conv, i64 42, double %1)
+  ret void
----------------
cebowleratibm wrote:
> hubert.reinterpretcast wrote:
> > cebowleratibm wrote:
> > > hubert.reinterpretcast wrote:
> > > > cebowleratibm wrote:
> > > > > ZarkoCA wrote:
> > > > > > Can one of these be set as a float? This way it's explicitly shown that floats and doubles are handled differently. 
> > > > > We could, but there's no way to do so from C code.  You can do so for typed functions, but then you don't initialize the GPR and this is outside the scope of this patch.
> > > > > 
> > > > > We could write such a test in IR with understanding nothing in C/C++ will produce such IR.
> > > > This case suggested by @ZarkoCA raises the question of how a 32-bit floating point value will be represented in a 64-bit GPR. What is the expected result? The equivalent case for XL generates a 64-bit GPR with zeroes in the high bits and the bits forming the 32-bit floating point value in the low bits.
> > > I agree there is a question of how an f32 occupies a 64-bit register, though this issue is not specific to calling convention and I had considered it outside the scope of this patch.
> > > 
> > > I'm adding an assembly test that the GPRs can be initialized without going through memory on newer hardware.  I'll also add an assembly test for passing an f32 to a typed function and validate the instructions used to populate the register.
> > As you said, passing f32 to a typed function would not populate a 64-bit GPR. The patch does, however, admit the possibility of passing f32 in a vararg case under 64-bit mode, hence the question. Even if the patch is updated to error on any case of needing to populate a GPR from an f32, the test should probably test for the error.
> > 
> > In any case, it is known that the treatment of `float _Complex` in Clang IR codegen is not correct on AIX (the byval struct passing would populate 64 bits of GPR and no FPRs). The passing of `float _Complex` via varargs is a case where we populate GPRs with f32s in the manner I described.
> I was confused on my last comment.  We're talking about the 64-bit GPR initialization and this issue is specific to vararg calling convention.  I'll make f32 vararg an error for both 32 and 64 bit modes.  The 32-bit mode case probably doesn't need to be an error but I think a follow on patch can address it and the _Complex case you mention.
In the most recent revision I decided it was just as well to make f32 vararg work.  Test added as call_test_vararg4.


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

https://reviews.llvm.org/D71013