[libc-commits] [PATCH] D158867: [libc] Add more test cases to the argument list tests

[Jon Chesterfield via Phabricator via libc-commits]

JonChesterfield added a comment.

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



================
Comment at: libc/test/src/__support/arg_list_test.cpp:134
+  va_start(vlist, first);
+  __llvm_libc::internal::ArgList args(vlist);
+  va_end(vlist);
----------------
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?


================
Comment at: libc/test/src/__support/arg_list_test.cpp:77
+  count += args.next_var<double>();
+  count += args.next_var<long>();
+  if (args.next_var<void *>() != nullptr)
----------------
jhuber6 wrote:
> arsenm wrote:
> > jhuber6 wrote:
> > > arsenm wrote:
> > > > jhuber6 wrote:
> > > > > arsenm wrote:
> > > > > > Also should test short, half, pointers, ext_vector_type, bool
> > > > > Would `ext_vector_type` compile on the GPU?
> > > > well it's all the vector types. that's what opencl vectors are and work
> > > 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).
> > No, it's the clang extension used to implement opencl, just through typedefs. e.g.
> > 
> > ```
> > typedef int int4 __attribute__((ext_vector_type(4)));
> > ```
> Seems that's a clang extension, so we'll probably need to guard it somehow.
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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158867