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

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added a comment.

Overall I think this patch looks great.

I've only added one comment. It relates to the way that a test case has changed when a struct is being passed in as a parameter.
I know that this is work in progress and it is quite possible that passing structs is not yet supported. If that's the case just add a comment (or TODO) into the test to show that the results are wrong and we know they are wrong and that they will be fixed at a later date.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:18213
   default:
-    return CC_PPC64_ELF_FIS;
+    return CC_PPC64_ELF;
   }
----------------
amyk wrote:
> stefanp wrote:
> > Does this also need to be changed in PPCFastISel? 
> Just wanted to double check, do you mean if I need to update the calling convention used within `PPCFastISel` from `CC_PPC64_ELF_FIS` -> `CC_PPC64_ELF`? 
> 
> If so, that’s actually a good point. I see that the FastISel TableGen definition is only a super simple one that handles ints and floats, and this one is supposed to be a more full implementation. 
> 
> I actually did a quick test to see if anything went wrong if I removed the old FastISel definitions and replaced them with these ones. The testing came out clean and had no issues, so perhaps it’s a possibility that I could update the PPCFastISel one, as well. Or, I can probably put up a separate NFC patch afterwards to update this?
I'm happy if this is a separate patch that comes after or even as a TODO in the code for us to look at later. 


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137504