[PATCH] D137504: [PowerPC] Implement 64-bit ELFv2 Calling Convention in TableGen (for integers/floats/vectors in registers)

Fri Jan 27 14:17:24 PST 2023

amyk added inline comments.

================
Comment at: llvm/test/CodeGen/PowerPC/GlobalISel/irtranslator-args-lowering.ll:79
   ; CHECK: bb.1.entry:
-  ; CHECK:   liveins: $f1, $x3, $x4, $x5, $x6
+  ; CHECK:   liveins: $f1, $x3, $x5, $x6, $x7
   ; CHECK:   [[COPY:%[0-9]+]]:_(s64) = COPY $x3
----------------
stefanp wrote:
> This is interesting because we are changing the way that a struct is being passed in registers.
> Which of these set of liveins is correct?
> 
> My understanding of the ABI (and I could be wrong here so please read and make sure) is that non-homogeneous structs are passed as a block of memory. For example the `i8` and `float` would be in `R3` at the same time.
> So, it might look something like this:
> ```
> R3 -> i8, float
> R4 -> i32, i32
> R5 -> i32
> ``` 
>  If we look at it that way we should only be using `R3`, `R4`, `R5`.
> 
> Either way, I think it is important to look at this and figure out why it changes and what the ABI says.
I count be misunderstanding this as well but when reading the ABI initially, I understood it the same as you described in your comment where we would be utilizing r2, r4 and r5 for this particular struct. 

Essentially, I thought that both the original and updated `liveins` is incorrect, just because this implementation is meant to handle simple cases of integers, floats and vectors within registers and doesn't fully support structs yet. I thought I had put a comment denoting this before but it turns out that I didn't, so thank you for pointing this out, Stefan. 

I can definitely add the TODO here, and we can plan to add support for structs in a follow up patch at a later time. I also just wanted to check with @nemanjai if this is a reasonable approach for this patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137504