[libc] Add more test cases to the argument list tests

[jhuber6]

Summary

This patch adds some extra cases to the existing argument list test in
libc, mainly dealing with arguments of varying sizes and primitive
types.

The purpose of this patch is to provide a wider test area when we begin
to provide varargs support on the GPU as there is no other runtime code
that really tests it. So, running these tests more exhaustively in the
GPU libc project will serve as the runtime tests for GPU vararg support in
D158246.

[jhuber6]

Adjust test

[arsenm]

libc/test/src/__support/arg_list_test.cpp

67

no char either?

77

Also should test short, half, pointers, ext_vector_type, bool

[jhuber6]

libc/test/src/__support/arg_list_test.cpp

67

It's passed in as a char, but all varargs get promoted to an integer as far as I'm aware, https://godbolt.org/z/Wssrrz3M1.

77

Would ext_vector_type compile on the GPU?

[arsenm]

libc/test/src/__support/arg_list_test.cpp

77

well it's all the vector types. that's what opencl vectors are and work

[jhuber6]

libc/test/src/__support/arg_list_test.cpp

77

Can you provide an example of that? I assume ext_vector_specific is an OpenCL term that might has some other way to use externally like a lot of other things. This test should primarily work on X64 but we can use macros to only enable a test for the GPU if needed (though it won't do anything right now).

[arsenm]

libc/test/src/__support/arg_list_test.cpp

77

No, it's the clang extension used to implement opencl, just through typedefs. e.g.

typedef int int4 __attribute__((ext_vector_type(4)));

[jhuber6]

libc/test/src/__support/arg_list_test.cpp

77

Seems that's a clang extension, so we'll probably need to guard it somehow.

[jhuber6]

Fix test and add a vector type test if using clang.

[JonChesterfield]

Thank you for adding this! There's much reassurance to be had from knowing the variadic lowering produces code that executes as expected.

Does anything here check that the values have made it through unchanged? I see counting how often next_var was called, but not checking that values aren't mangled in transit

libc/test/src/__support/arg_list_test.cpp

77

There are a few vector types, different variations on syntax over the IR representation. Any one should do here - has_attribute or similar will suffice as a guard I think

134

Does this call va_copy on the argument? I.e. is va_end immediately after making one the safe thing to do, even if va_end is not a no-op?

[jhuber6]

Change to add LIBC_HAS_ATTRIBUTE and use separate values.

[jhuber6]

libc/test/src/__support/arg_list_test.cpp

134

Yes, it va_copy's it for the constructor. I think there's an explicit test for that in this file.

[sivachandra]

OK with some minor comments inline.

libc/src/__support/macros/config.h

44 ↗

(On Diff #553613)

s/__has_feature/__has_attribute

libc/test/src/__support/arg_list_test.cpp

90

Can we avoid non-primitives and just stick to primitives here? For example, instead of intmax_t, size_t and ptrdiff_t, add unsigned short etc? Then, we don't have to worry about platforms where size_t is not long long etc.

[jhuber6]

libc/src/__support/macros/config.h

44 ↗

(On Diff #553613)

Whoops, thanks for catching that.

libc/test/src/__support/arg_list_test.cpp

90

Will do.

libc/test/src/__support/arg_list_test.cpp

 
 TEST(LlvmLibcArgListTest, CopyConstructor) {
   ASSERT_EQ(sum_two_nums(3, 1, 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024),
             10);
 
   ASSERT_EQ(sum_two_nums(3, 5, 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024),
             40);
 }
+
+bool check_primitives(int first, ...) {
+  va_list vlist;
+  va_start(vlist, first);
+  __llvm_libc::internal::ArgList args(vlist);
+  va_end(vlist);
+
+  if (args.next_var<int>() != 0)
+    return false;
+  if (args.next_var<int>() != 0)
+    return false;
+  if (args.next_var<int>() != 0)
+    return false;
+  if (args.next_var<long>() != 0)
+    return false;
+  if (args.next_var<long>() != 0)
+    return false;
+  if (args.next_var<intmax_t>() != 0)
+    return false;
+  if (args.next_var<size_t>() != 0)
+    return false;
+  if (args.next_var<ptrdiff_t>() != 0)
+    return false;
+  if (args.next_var<double>() != 0)
+    return false;
+  if (args.next_var<double>() != 0)
+    return false;
+  if (args.next_var<long>() != 0)
+    return false;
+  if (args.next_var<void *>() != 0)
+    return false;
+  return true;
+}
+
+TEST(LlvmLibcArgListTest, TestPrimitiveTypes) {
+  char x1 = 0;
+  short x2 = 0;
+  int x3 = 0;
+  long x4 = 0;
+  long long x5 = 0;
+  intmax_t x6 = 0;
+  size_t x7 = 0;
+  ptrdiff_t x8 = 0;
+  float x9 = 0;
+  double x10 = 0;
+  long double x11 = 0;
+  void *x12 = 0;
+  ASSERT_TRUE(
+      check_primitives(0, x1, x2, x3, x4, x5, x6, x7, x8, x9, x10, x11, x12));
+}
+
+struct S {
+  char c;
+  short s;
+  int i;
+  long l;
+  float f;
+  double d;
+};
+
+long int check_struct_type(int first, ...) {
+  va_list vlist;
+  va_start(vlist, first);
+  __llvm_libc::internal::ArgList args(vlist);
+  va_end(vlist);
+
+  S s = args.next_var<S>();
+  int last = args.next_var<int>();
+  return s.c + s.s + s.i + s.l + s.f + s.d + last;
+}
+
+TEST(LlvmLibcArgListTest, TestStructTypes) {
+  S s{'\x1', 1, 1, 1l, 1.0f, 1.0};
+  ASSERT_EQ(check_struct_type(0, s, 1), 7l);
+}