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

[Amy Kwan via Phabricator via llvm-commits]

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
----------------
nemanjai wrote:
> amyk wrote:
> > 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.
> One easy way to resolve this is to compile something with current compilers that accesses the members from Stefan's example and see which register they're expected to be in.
> 
> Also, if we happen to do the wrong thing for GlobalISel for now for passing structs, I'm perfectly fine with marking it as a TODO to fix it later.
Sounds good. I have added a TODO when I committed the patch. The correct way appears to be what Stefan has outlined/what was discussed (R3, R4, R5).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137504