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

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

LGTM.



================
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
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137504