[PATCH] D152752: [MS] Fix passing aligned records by value in some cases

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Intel is seeing some fallout from this change in our downstream (reverting this commit causes our test case to pass). Our test looks like this:

  #include <stdlib.h>
  #include <stdio.h>
  #include <stdarg.h>
  #include <x86intrin.h>
  
  typedef union {
  	char c[32];
  	short s[16];
  	int d[8];
  	long long l[4];
  	__m128 h[2];
  	__m256 m;
  	__m256i mi;
  	__m256d md;
  } M256;
  
  M256 make_m256_from_int(int d1, int d2, int d3, int d4, int d5, int d6, int d7, int d8) {
  	M256 ret;
  	ret.d[0] = d1;
  	ret.d[1] = d2;
  	ret.d[2] = d3;
  	ret.d[3] = d4;
  	ret.d[4] = d5;
  	ret.d[5] = d6;
  	ret.d[6] = d7;
  	ret.d[7] = d8;
  	return ret;
  }
  
  typedef union {
  	char c;
  } M8;
  
  M8 make_m8_from_char(char c) {
  	M8 ret;
  	ret.c = c;
  	return ret;
  }
  
  int test(char* format, ...) {
  	int i, j;
  	M256 m256;
  	M8 m8;
  	int retValue = 0;
  	va_list args;
  	va_start(args, format);
  
  	for (i = 0; format[i]; i++) {
  		switch (format[i]) {
  			case '1':
  				m8 = va_arg(args, M8);
  				retValue += m8.c;
  				break;
  			case '6':
  				m256 = va_arg(args, M256);
  				retValue += m256.d[0] + m256.d[1] + m256.d[2] + m256.d[3] + m256.d[4] + m256.d[5] + m256.d[6] + m256.d[7];
  				break;
  		}
  	}
  
  	va_end(args);
  	return retValue;
  }
  
  int main(void) {
  	int retValue = 0;
  
  	if (test("16", make_m8_from_char(-12), make_m256_from_int(1, 2, 1, 2, 1, 2, 1, 2))) {
  		fprintf(stderr, "test failed for: M8(%d) M256(%d,%d,%d,%d,%d,%d,%d,%d)\n", 12, 1, 2, 1, 2, 1, 2, 1, 2);
  		retValue++;
  	}
  
  	if (retValue) {
  		printf("FAILED %d tests\n", retValue);
  	} else {
  		printf("PASSED\n");
  	}
  	return retValue;
  }

What we're seeing is the difference in IR emitted by FE is the byval attribute and alignment for 3rd argument.

Bad

  %call2 = call i32 (ptr, ...) @test(ptr noundef @"??_C at _02KMALDIDP@16?$AA@", ptr noundef byval(%union.M8) align 4 %agg.tmp1, ptr noundef %agg.tmp), !dbg !237

Good

  %call2 = call i32 (ptr, ...) @test(ptr noundef @"??_C at _02KMALDIDP@16?$AA@", ptr noundef byval(%union.M8) align 4 %agg.tmp1, ptr noundef byval(%union.M256) align 4 %agg.tmp), !dbg !237

(We're seeing it with this clang-cl-esque command line: `icx -c  -Zi -Od  /arch:AVX check_types_reduced.c -Xclang -emit-llvm -Xclang -disable-llvm-passes`)

I didn't spot any UB in the test, so I think something may be incorrect with this patch (but codegen is not my area of specialty, so please correct me if I'm wrong).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152752