[PATCH] D133920: [X86][fastcall][vectorcall] Move capability check before free register update

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Thanks, I have more comments, sorry I didn't add them on the first review.



================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:1785
 
-  if (State.CC == llvm::CallingConv::X86_FastCall ||
-      State.CC == llvm::CallingConv::X86_VectorCall ||
-      State.CC == llvm::CallingConv::X86_RegCall) {
-    if (getContext().getTypeSize(Ty) > 32)
-      return false;
+  if (!IsPtrOrInt && State.CC == llvm::CallingConv::X86_RegCall)
+    return false;
----------------
I see, no intended behavior change for regcall.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:1788-1791
+  if (IsMCUABI)
+    return false;
 
   return true;
----------------
Seems like this can be simplified to:
  // Return true to apply inreg to all legal parameters except on for MCU targets.
  return !IsMCUABI;


================
Comment at: clang/test/CodeGen/mangle-windows.c:50
 void __fastcall f9(long long a, char b, char c, short d) {}
-// CHECK: define dso_local x86_fastcallcc void @"\01 at f9@20"(i64 noundef %a, i8 noundef signext %b, i8 noundef signext %c, i16 noundef signext %d)
+// CHECK: define dso_local x86_fastcallcc void @"\01 at f9@20"(i64 noundef %a, i8 inreg noundef signext %b, i8 inreg noundef signext %c, i16 noundef signext %d)
 // X64: define dso_local void @f9(
----------------
This file is intended to test the C mangling. Most of the other tests don't look at the IR prototype. I think a better home for both tests is:
../clang/test/CodeGen/stdcall-fastcall.c
../clang/test/CodeGen/vectorcall.c

Do we already have coverage for this case for regcall? Please ensure we have coverage or add it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133920