[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 a comment.

@stefanp Thanks for the review and the questions!

> Can we now get rid of CC_PPC64_ELF_FIS completely?

That’s a good question! This is related to your question about updating PPCFastISel. From what I can tell, it looks like mine covers everything the PPCFastISel one covers:

  // Simple calling convention for 64-bit ELF PowerPC fast isel.
  // Only handle ints and floats.  All ints are promoted to i64.
  // Vector types and quadword ints are not handled.
  let Entry = 1 in
  def CC_PPC64_ELF_FIS : CallingConv<[
    CCIfCC<“CallingConv::AnyReg”, CCDelegateTo<CC_PPC64_AnyReg>>,
  
    CCIfType<[i1],  CCPromoteToType<i64>>,
    CCIfType<[i8],  CCPromoteToType<i64>>,
    CCIfType<[i16], CCPromoteToType<i64>>,
    CCIfType<[i32], CCPromoteToType<i64>>,
    CCIfType<[i64], CCAssignToReg<[X3, X4, X5, X6, X7, X8, X9, X10]>>,
    CCIfType<[f32, f64], CCAssignToReg<[F1, F2, F3, F4, F5, F6, F7, F8]>>
  ]>;

Although I realize there is a separate return value one that handles `i128` that this patch does not yet handle:

  // Simple return-value convention for 64-bit ELF PowerPC fast isel.
  // All small ints are promoted to i64.  Vector types, quadword ints,
  // and multiple register returns are “supported” to avoid compile
  // errors, but none are handled by the fast selector.
  let Entry = 1 in
  def RetCC_PPC64_ELF_FIS : CallingConv<[
    CCIfCC<“CallingConv::AnyReg”, CCDelegateTo<RetCC_PPC64_AnyReg>>,
  
    CCIfType<[i1],   CCPromoteToType<i64>>,
    CCIfType<[i8],   CCPromoteToType<i64>>,
    CCIfType<[i16],  CCPromoteToType<i64>>,
    CCIfType<[i32],  CCPromoteToType<i64>>,
    CCIfType<[i64],  CCAssignToReg<[X3, X4, X5, X6]>>,
    CCIfType<[i128], CCAssignToReg<[X3, X4, X5, X6]>>,
    CCIfType<[f32],  CCAssignToReg<[F1, F2, F3, F4, F5, F6, F7, F8]>>,
    CCIfType<[f64],  CCAssignToReg<[F1, F2, F3, F4, F5, F6, F7, F8]>>,
    CCIfType<[f128],
             CCIfSubtarget<“hasAltivec()“,
             CCAssignToReg<[V2, V3, V4, V5, V6, V7, V8, V9]>>>,
    CCIfType<[v16i8, v8i16, v4i32, v2i64, v1i128, v4f32, v2f64],
             CCIfSubtarget<“hasAltivec()“,
             CCAssignToReg<[V2, V3, V4, V5, V6, V7, V8, V9]>>>
  ]>;

As I mentioned in one of my replies to your other comments, I did try to use this definition instead of the FastISel ones within `PPCFastISel.cpp` and there weren’t any issues.
It may be possible that we can get rid of `CC_PPC64_ELF_FIS`, although I’m not sure if we should do it within this patch. @nemanjai Do you have any thoughts on this?



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:18213
   default:
-    return CC_PPC64_ELF_FIS;
+    return CC_PPC64_ELF;
   }
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137504