LuoYuanke added inline comments.

================
Comment at: clang/include/clang/CodeGen/CGFunctionInfo.h:590
+  /// Log 2 of the maximum vector width.
+  unsigned MaxVectorWidth : 4;
+
----------------
I notice some code would indicate it is log 2 size with Log2 suffix in the 
variable name. Do you think it is more readable to add Log2 suffix?


================
Comment at: clang/lib/CodeGen/CGCall.cpp:5238
+  for (unsigned i = 0; i < IRCallArgs.size(); ++i)
+    LargestVectorWidth = std::max(LargestVectorWidth,
+                                  getMaxVectorWidth(IRCallArgs[i]->getType()));
----------------
Does this also affect other calling convention besides fastcall?


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:2303
   void classify(QualType T, uint64_t OffsetBase, Class &Lo, Class &Hi,
-                bool isNamedArg) const;
+                bool isNamedArg, bool IsRegCall = false) const;
 
----------------
Update the comments for the new parameter?


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:3031
     // than eight eightbytes, ..., it has class MEMORY.
-    if (Size > 512)
+    if (!IsRegCall && Size > 512)
       return;
----------------
Would you add a test for non regcall? Pass 1024 bit vector parameter and check 
if it is well handled both with regcall and without regcall.
Would you add comments to depict why regcall accept the size which is more than 
512?


================
Comment at: clang/test/CodeGen/aarch64-neon-tbl.c:45
 
-// CHECK-LABEL: define{{.*}} <8 x i8> @test_vqtbl2_s8([2 x <16 x i8>] 
%a.coerce, <8 x i8> noundef %b) #0 {
+// CHECK-LABEL: define{{.*}} <8 x i8> @test_vqtbl2_s8([2 x <16 x i8>] 
%a.coerce, <8 x i8> noundef %b) #1 {
 // CHECK:   [[__P0_I:%.*]] = alloca %struct.int8x16x2_t, align 16
----------------
I'm curious why aarch64 test cases are affected by the patch.


================
Comment at: clang/test/CodeGen/regcall2.c:2
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -emit-llvm %s -o - -ffreestanding -target-feature +avx512vl 
-triple=x86_64-pc-win32     | FileCheck %s --check-prefix=Win
+// RUN: %clang_cc1 -emit-llvm %s -o - -ffreestanding -target-feature +avx512vl 
-triple=x86_64-pc-linux-gnu | FileCheck %s --check-prefix=Lin
----------------
Add test case for target that has no avx512 feature?


================
Comment at: clang/test/CodeGen/regcall2.c:9
+  __m512d r1[4];
+  __m512 r2[4];
+} __sVector;
----------------
May add a test case to show what's the max register we can pass with regcall.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122104

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to