Add UnitTest to test-suite to check correct handling of large stack alignments

[kristof.beyls]

Summary

As part of creating a fix for PR13007, "ARM CodeGen fails with
large stack alignment", I created a test-suite UnitTest to check
correct execution of functions with various stack alignment
constraints. I think it could be worthwhile to add it to the
test-suite, although it may be a bit too specific a test case
to add as a UnitTest?
Overall, I think it's worthwhile to add it, but I'm interested
in finding out what others think.

Thanks,

Kristof

PS. The review for the partial fix I've got for PR13007 is at
http://reviews.llvm.org/D6844.

[rengolin]

Hi Kristof,

Is this test supposed to pass or fail on ARM? If pass, how can it test the big-stack issue?

I think that having an extra unit test is never a bad idea, if it tests something as serious as that, but we need to be careful to test platform-specific things right.

If the ranges and sizes vary with the architecture, the test should account for that. If the test is irrelevant in other architectures, it should be disabled on them.

cheers,
--renato

SingleSource/UnitTests/Makefile

141

TAB issue. TABs in Makefiles are magic, make sure not to insert spaces instead.

[kristof.beyls]

Thanks for noticing the tab vs spaces issue - I don't think it affected correctness in this case, it should be a tab just for consistency. Now corrected in updated patch.

The test is indeed failing on ARM targets, because of http://llvm.org/bugs/show_bug.cgi?id=13007.
http://reviews.llvm.org/D6844 is the review for a patch fixing 13007 (and making the test in this patch pass) for ARM and Thumb2 targets.

I didn't tune the sizes and alignments being tested for ARM specifically; the only function I added for ARM specifically is copy_and_check_aligned_stack_copy_scalar_spill, because ARMFrameLowering has a different code path emitting stack-aligning
instructions when floating point registers need to be spilled on a target with Neon instructions. I could eliminate that function if
it that seems to be too much "white box" instead of "black box" testing.

I don't know of LLVM having bugs for stack alignments for other architectures - but the test is valid for all architectures,
so if added, should be enabled for all architectures.

Thanks,

Kristof

[rengolin]

I see. Assuming you tested on at least x86_64 in addition to ARM and AArch64, LGTM, after the proper fix is in.

[rengolin]

Any progress in this? If not, can we close this review?

[kristof.beyls]

Testing this on AArch64 showed that dynamic stack re-alignment hasn't been implemented in the AArch64 backend - i.e. this test fails on AArch64.
I'm implementing support for that in the AArch64 backend - I hope to have a patch up for review soon, after which this test can be committed without breaking the AArch64 bots.

[rengolin]

I thought I saw some patches going in to fix the alignment... We should then wait until the fix has gone through. Thanks!

SingleSource/UnitTests/Makefile

PROGRAMS_TO_SKIP += \
   StructModifyTest \
   TestLoop \
   vla \
   ms_struct-bitfield \
   ms_struct-bitfield-1 \
   ms_struct-bitfield-init \
   ms_struct-bitfield-init-1 \
   ms_struct_pack_layout \
-  ms_struct_pack_layout-1
+  ms_struct_pack_layout-1 \
+  large_stack_alignment
 endif
 
 LDFLAGS += -lstdc++
 include $(LEVEL)/SingleSource/Makefile.singlesrc

SingleSource/UnitTests/large_stack_alignment.cpp

+#include <cstring>
+#include <cassert>
+#include <cstdio>
+
+static int return_value = 0;
+
+__attribute__((noinline))
+void check_aligned_copy(const char* variant,
+                        const unsigned int alignment, const unsigned size,
+                        const char* input, const char* copied_on_stack)
+{
+  bool alignment_as_expected =
+    ((std::ptrdiff_t)copied_on_stack & (((std::ptrdiff_t)alignment)-1)) == 0;
+  bool content_as_expected = std::memcmp(input, copied_on_stack, size) == 0;
+  printf("%s, alignment %5d:", variant, alignment);
+  if  (alignment_as_expected && content_as_expected)
+    printf(" OK");
+  else {
+    if (!alignment_as_expected)
+      printf(" ALIGNMENT NOT AS EXPECTED: %p", copied_on_stack);
+    if (!content_as_expected)
+      printf(" CONTENT NOT AS EXPECTED");
+  }
+  printf("\n");
+  if (!(alignment_as_expected && content_as_expected))
+    return_value = -1;
+}
+
+template<int alignment>
+__attribute__((noinline))
+void copy_and_check_aligned_stack_copy_vla(unsigned size, const char* input) {
+    int res;
+    char aligned_buffer[size] __attribute__((__aligned__(alignment)));
+    for(unsigned i=0;i<size;++i)
+        aligned_buffer[i] = input[i];
+    check_aligned_copy("vla", alignment, size, input, aligned_buffer);
+}
+
+template<int alignment, unsigned size>
+__attribute__((noinline))
+void copy_and_check_aligned_stack_copy_fla(const char* input) {
+    int res;
+    char aligned_buffer[size] __attribute__((__aligned__(alignment)));
+    for(unsigned i=0;i<size;++i)
+        aligned_buffer[i] = input[i];
+    check_aligned_copy("fla", alignment, size, input, aligned_buffer);
+}
+
+template<int alignment>
+__attribute__((noinline))
+void copy_and_check_aligned_stack_copy_scalar(const char* input) {
+    int res;
+    char aligned_buffer __attribute__((__aligned__(alignment)));
+    aligned_buffer = input[0];
+    check_aligned_copy("scalar", alignment, 1, input, &aligned_buffer);
+}
+
+template<int alignment>
+__attribute__((noinline))
+double copy_and_check_aligned_stack_copy_scalar_spill(const char* input) {
+    char aligned_buffer __attribute__((__aligned__(alignment)));
+    aligned_buffer = input[0];
+    check_aligned_copy("scalar", alignment, 1, input, &aligned_buffer);
+#define DXX(prev_number, number) const double d##number = d1*d##prev_number
+    const double d1 = (double)input[0];
+                DXX( 1, 2); DXX( 2, 3); DXX( 3, 4); DXX( 4, 5); DXX( 5, 6); DXX( 6, 7); DXX( 7, 8); DXX( 8, 9); DXX( 9,10);
+    DXX(10,11); DXX(11,12); DXX(12,13); DXX(13,14); DXX(14,15); DXX(15,16); DXX(16,17); DXX(17,18); DXX(18,19); DXX(19,20);
+    DXX(20,21); DXX(21,22); DXX(22,23); DXX(23,24); DXX(24,25); DXX(25,26); DXX(26,27); DXX(27,28); DXX(28,29); DXX(29,30);
+    DXX(30,31); DXX(31,32); DXX(32,33); DXX(33,34); DXX(34,35); DXX(35,36); DXX(36,37); DXX(37,38); DXX(38,39);
+    return (  d1/ d2/ d3/ d4/ d5/ d6/ d7/ d8/ d9/d10
+            /d11/d12/d13/d14/d15/d16/d17/d18/d19/d20
+            /d21/d22/d23/d24/d25/d26/d27/d28/d29/d30
+            /d31/d32/d33/d34/d35/d36/d37/d38/d39)
+         *( d39/d38/d37/d36/d35/d34/d33/d32/d31/d30
+           /d29/d28/d27/d26/d25/d24/d23/d22/d21/d20
+           /d19/d18/d17/d16/d15/d14/d13/d12/d11/d10
+           / d9/ d8/ d7/ d6/ d5/ d4/ d3/ d2/ d1);
+}
+
+
+
+
+int main() {
+  copy_and_check_aligned_stack_copy_vla<   1>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla<   2>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla<   4>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla<   8>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla<  16>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla<  32>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla<  64>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla< 128>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla< 256>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla< 512>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla<1024>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla<2048>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla<4096>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_vla<8192>(10, "abcdefghij");
+  copy_and_check_aligned_stack_copy_fla<   1,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla<   2,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla<   4,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla<   8,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla<  16,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla<  32,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla<  64,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla< 128,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla< 256,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla< 512,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla<1024,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla<2048,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla<4096,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_fla<8192,10>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar<   1>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar<   2>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar<   4>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar<   8>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar<  16>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar<  32>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar<  64>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar< 128>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar< 256>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar< 512>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar<1024>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar<2048>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar<4096>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar<8192>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill<   1>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill<   2>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill<   4>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill<   8>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill<  16>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill<  32>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill<  64>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill< 128>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill< 256>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill< 512>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill<1024>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill<2048>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill<4096>("abcdefghij");
+  copy_and_check_aligned_stack_copy_scalar_spill<8192>("abcdefghij");
+  return return_value;
+}

SingleSource/UnitTests/large_stack_alignment.reference_output

+vla, alignment     1: OK
+vla, alignment     2: OK
+vla, alignment     4: OK
+vla, alignment     8: OK
+vla, alignment    16: OK
+vla, alignment    32: OK
+vla, alignment    64: OK
+vla, alignment   128: OK
+vla, alignment   256: OK
+vla, alignment   512: OK
+vla, alignment  1024: OK
+vla, alignment  2048: OK
+vla, alignment  4096: OK
+vla, alignment  8192: OK
+fla, alignment     1: OK
+fla, alignment     2: OK
+fla, alignment     4: OK
+fla, alignment     8: OK
+fla, alignment    16: OK
+fla, alignment    32: OK
+fla, alignment    64: OK
+fla, alignment   128: OK
+fla, alignment   256: OK
+fla, alignment   512: OK
+fla, alignment  1024: OK
+fla, alignment  2048: OK
+fla, alignment  4096: OK
+fla, alignment  8192: OK
+scalar, alignment     1: OK
+scalar, alignment     2: OK
+scalar, alignment     4: OK
+scalar, alignment     8: OK
+scalar, alignment    16: OK
+scalar, alignment    32: OK
+scalar, alignment    64: OK
+scalar, alignment   128: OK
+scalar, alignment   256: OK
+scalar, alignment   512: OK
+scalar, alignment  1024: OK
+scalar, alignment  2048: OK
+scalar, alignment  4096: OK
+scalar, alignment  8192: OK
+scalar, alignment     1: OK
+scalar, alignment     2: OK
+scalar, alignment     4: OK
+scalar, alignment     8: OK
+scalar, alignment    16: OK
+scalar, alignment    32: OK
+scalar, alignment    64: OK
+scalar, alignment   128: OK
+scalar, alignment   256: OK
+scalar, alignment   512: OK
+scalar, alignment  1024: OK
+scalar, alignment  2048: OK
+scalar, alignment  4096: OK
+scalar, alignment  8192: OK
+exit 0