[clang][CodeGen] Use byval for SystemZ indirect arguments #66404
Conversation
SystemZ ABI mandates that in order to pass large objects by value, one should be place them on stack and pass the callee a pointer. Currently on LLVM IR level it's impossible to distinguish whether a pointer argument was there in the C code, or whether it was introduced in order to satisfy the ABI requirement. MSan and DFSan need to know that in order to correctly fill the arguments' shadow TLS. Fix by adding ByVal attributes to synthetic pointer arguments. This attribute serves exactly this purpose. This resolves an outstanding MSan issue, and enables the future DFSan support.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang ChangesSystemZ ABI mandates that in order to pass large objects by value, one should be place them on stack and pass the callee a pointer. Currently on LLVM IR level it's impossible to distinguish whether a pointer argument was there in the C code, or whether it was introduced in order to satisfy the ABI requirement.MSan and DFSan need to know that in order to correctly fill the arguments' shadow TLS. Fix by adding ByVal attributes to synthetic pointer arguments. This attribute serves exactly this purpose. This resolves an outstanding MSan issue, and enables the future DFSan support.Patch is 22.42 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/66404.diff 6 Files Affected:
diff --git a/clang/lib/CodeGen/Targets/SystemZ.cpp b/clang/lib/CodeGen/Targets/SystemZ.cpp
index 6eb0c6ef2f7d637..3a0177cfc73446c 100644
--- a/clang/lib/CodeGen/Targets/SystemZ.cpp
+++ b/clang/lib/CodeGen/Targets/SystemZ.cpp
@@ -432,7 +432,7 @@ ABIArgInfo SystemZABIInfo::classifyArgumentType(QualType Ty) const {
// Values that are not 1, 2, 4 or 8 bytes in size are passed indirectly.
if (Size != 8 && Size != 16 && Size != 32 && Size != 64)
- return getNaturalAlignIndirect(Ty, /*ByVal=*/false);
+ return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
// Handle small structures.
if (const RecordType *RT = Ty->getAs<RecordType>()) {
@@ -440,7 +440,7 @@ ABIArgInfo SystemZABIInfo::classifyArgumentType(QualType Ty) const {
// fail the size test above.
const RecordDecl *RD = RT->getDecl();
if (RD->hasFlexibleArrayMember())
- return getNaturalAlignIndirect(Ty, /*ByVal=*/false);
+ return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
// The structure is passed as an unextended integer, a float, or a double.
llvm::Type *PassTy;
@@ -457,7 +457,7 @@ ABIArgInfo SystemZABIInfo::classifyArgumentType(QualType Ty) const {
// Non-structure compounds are passed indirectly.
if (isCompoundType(Ty))
- return getNaturalAlignIndirect(Ty, /*ByVal=*/false);
+ return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
return ABIArgInfo::getDirect(nullptr);
}
diff --git a/clang/test/CodeGen/SystemZ/systemz-abi-vector.c b/clang/test/CodeGen/SystemZ/systemz-abi-vector.c
index f7606641a3747e5..764d3f04140b694 100644
--- a/clang/test/CodeGen/SystemZ/systemz-abi-vector.c
+++ b/clang/test/CodeGen/SystemZ/systemz-abi-vector.c
@@ -54,91 +54,91 @@ unsigned int align = __alignof__ (v16i8);
// CHECK-VECTOR: @align ={{.*}} global i32 8
v1i8 pass_v1i8(v1i8 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v1i8(ptr noalias sret(<1 x i8>) align 1 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v1i8(ptr noalias sret(<1 x i8>) align 1 %{{.*}}, ptr byval(<1 x i8>) align 1 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <1 x i8> @pass_v1i8(<1 x i8> %{{.*}})
v2i8 pass_v2i8(v2i8 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v2i8(ptr noalias sret(<2 x i8>) align 2 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v2i8(ptr noalias sret(<2 x i8>) align 2 %{{.*}}, ptr byval(<2 x i8>) align 2 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <2 x i8> @pass_v2i8(<2 x i8> %{{.*}})
v4i8 pass_v4i8(v4i8 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v4i8(ptr noalias sret(<4 x i8>) align 4 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v4i8(ptr noalias sret(<4 x i8>) align 4 %{{.*}}, ptr byval(<4 x i8>) align 4 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <4 x i8> @pass_v4i8(<4 x i8> %{{.*}})
v8i8 pass_v8i8(v8i8 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v8i8(ptr noalias sret(<8 x i8>) align 8 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v8i8(ptr noalias sret(<8 x i8>) align 8 %{{.*}}, ptr byval(<8 x i8>) align 8 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <8 x i8> @pass_v8i8(<8 x i8> %{{.*}})
v16i8 pass_v16i8(v16i8 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v16i8(ptr noalias sret(<16 x i8>) align 16 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v16i8(ptr noalias sret(<16 x i8>) align 16 %{{.*}}, ptr byval(<16 x i8>) align 16 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <16 x i8> @pass_v16i8(<16 x i8> %{{.*}})
v32i8 pass_v32i8(v32i8 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v32i8(ptr noalias sret(<32 x i8>) align 32 %{{.*}}, ptr %0)
-// CHECK-VECTOR-LABEL: define{{.*}} void @pass_v32i8(ptr noalias sret(<32 x i8>) align 8 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v32i8(ptr noalias sret(<32 x i8>) align 32 %{{.*}}, ptr byval(<32 x i8>) align 32 %0)
+// CHECK-VECTOR-LABEL: define{{.*}} void @pass_v32i8(ptr noalias sret(<32 x i8>) align 8 %{{.*}}, ptr byval(<32 x i8>) align 8 %0)
v1i16 pass_v1i16(v1i16 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v1i16(ptr noalias sret(<1 x i16>) align 2 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v1i16(ptr noalias sret(<1 x i16>) align 2 %{{.*}}, ptr byval(<1 x i16>) align 2 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <1 x i16> @pass_v1i16(<1 x i16> %{{.*}})
v2i16 pass_v2i16(v2i16 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v2i16(ptr noalias sret(<2 x i16>) align 4 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v2i16(ptr noalias sret(<2 x i16>) align 4 %{{.*}}, ptr byval(<2 x i16>) align 4 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <2 x i16> @pass_v2i16(<2 x i16> %{{.*}})
v4i16 pass_v4i16(v4i16 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v4i16(ptr noalias sret(<4 x i16>) align 8 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v4i16(ptr noalias sret(<4 x i16>) align 8 %{{.*}}, ptr byval(<4 x i16>) align 8 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <4 x i16> @pass_v4i16(<4 x i16> %{{.*}})
v8i16 pass_v8i16(v8i16 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v8i16(ptr noalias sret(<8 x i16>) align 16 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v8i16(ptr noalias sret(<8 x i16>) align 16 %{{.*}}, ptr byval(<8 x i16>) align 16 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <8 x i16> @pass_v8i16(<8 x i16> %{{.*}})
v1i32 pass_v1i32(v1i32 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v1i32(ptr noalias sret(<1 x i32>) align 4 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v1i32(ptr noalias sret(<1 x i32>) align 4 %{{.*}}, ptr byval(<1 x i32>) align 4 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <1 x i32> @pass_v1i32(<1 x i32> %{{.*}})
v2i32 pass_v2i32(v2i32 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v2i32(ptr noalias sret(<2 x i32>) align 8 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v2i32(ptr noalias sret(<2 x i32>) align 8 %{{.*}}, ptr byval(<2 x i32>) align 8 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <2 x i32> @pass_v2i32(<2 x i32> %{{.*}})
v4i32 pass_v4i32(v4i32 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v4i32(ptr noalias sret(<4 x i32>) align 16 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v4i32(ptr noalias sret(<4 x i32>) align 16 %{{.*}}, ptr byval(<4 x i32>) align 16 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <4 x i32> @pass_v4i32(<4 x i32> %{{.*}})
v1i64 pass_v1i64(v1i64 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v1i64(ptr noalias sret(<1 x i64>) align 8 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v1i64(ptr noalias sret(<1 x i64>) align 8 %{{.*}}, ptr byval(<1 x i64>) align 8 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <1 x i64> @pass_v1i64(<1 x i64> %{{.*}})
v2i64 pass_v2i64(v2i64 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v2i64(ptr noalias sret(<2 x i64>) align 16 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v2i64(ptr noalias sret(<2 x i64>) align 16 %{{.*}}, ptr byval(<2 x i64>) align 16 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <2 x i64> @pass_v2i64(<2 x i64> %{{.*}})
v1i128 pass_v1i128(v1i128 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v1i128(ptr noalias sret(<1 x i128>) align 16 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v1i128(ptr noalias sret(<1 x i128>) align 16 %{{.*}}, ptr byval(<1 x i128>) align 16 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <1 x i128> @pass_v1i128(<1 x i128> %{{.*}})
v1f32 pass_v1f32(v1f32 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v1f32(ptr noalias sret(<1 x float>) align 4 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v1f32(ptr noalias sret(<1 x float>) align 4 %{{.*}}, ptr byval(<1 x float>) align 4 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <1 x float> @pass_v1f32(<1 x float> %{{.*}})
v2f32 pass_v2f32(v2f32 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v2f32(ptr noalias sret(<2 x float>) align 8 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v2f32(ptr noalias sret(<2 x float>) align 8 %{{.*}}, ptr byval(<2 x float>) align 8 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <2 x float> @pass_v2f32(<2 x float> %{{.*}})
v4f32 pass_v4f32(v4f32 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v4f32(ptr noalias sret(<4 x float>) align 16 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v4f32(ptr noalias sret(<4 x float>) align 16 %{{.*}}, ptr byval(<4 x float>) align 16 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <4 x float> @pass_v4f32(<4 x float> %{{.*}})
v1f64 pass_v1f64(v1f64 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v1f64(ptr noalias sret(<1 x double>) align 8 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v1f64(ptr noalias sret(<1 x double>) align 8 %{{.*}}, ptr byval(<1 x double>) align 8 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <1 x double> @pass_v1f64(<1 x double> %{{.*}})
v2f64 pass_v2f64(v2f64 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v2f64(ptr noalias sret(<2 x double>) align 16 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v2f64(ptr noalias sret(<2 x double>) align 16 %{{.*}}, ptr byval(<2 x double>) align 16 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <2 x double> @pass_v2f64(<2 x double> %{{.*}})
v1f128 pass_v1f128(v1f128 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_v1f128(ptr noalias sret(<1 x fp128>) align 16 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_v1f128(ptr noalias sret(<1 x fp128>) align 16 %{{.*}}, ptr byval(<1 x fp128>) align 16 %0)
// CHECK-VECTOR-LABEL: define{{.*}} <1 x fp128> @pass_v1f128(<1 x fp128> %{{.*}})
@@ -166,13 +166,13 @@ struct agg_v8i8 pass_agg_v8i8(struct agg_v8i8 arg) { return arg; }
struct agg_v16i8 { v16i8 a; };
struct agg_v16i8 pass_agg_v16i8(struct agg_v16i8 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_agg_v16i8(ptr noalias sret(%struct.agg_v16i8) align 16 %{{.*}}, ptr %{{.*}})
+// CHECK-LABEL: define{{.*}} void @pass_agg_v16i8(ptr noalias sret(%struct.agg_v16i8) align 16 %{{.*}}, ptr byval(%struct.agg_v16i8) align 16 %{{.*}})
// CHECK-VECTOR-LABEL: define{{.*}} void @pass_agg_v16i8(ptr noalias sret(%struct.agg_v16i8) align 8 %{{.*}}, <16 x i8> %{{.*}})
struct agg_v32i8 { v32i8 a; };
struct agg_v32i8 pass_agg_v32i8(struct agg_v32i8 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_agg_v32i8(ptr noalias sret(%struct.agg_v32i8) align 32 %{{.*}}, ptr %{{.*}})
-// CHECK-VECTOR-LABEL: define{{.*}} void @pass_agg_v32i8(ptr noalias sret(%struct.agg_v32i8) align 8 %{{.*}}, ptr %{{.*}})
+// CHECK-LABEL: define{{.*}} void @pass_agg_v32i8(ptr noalias sret(%struct.agg_v32i8) align 32 %{{.*}}, ptr byval(%struct.agg_v32i8) align 32 %{{.*}})
+// CHECK-VECTOR-LABEL: define{{.*}} void @pass_agg_v32i8(ptr noalias sret(%struct.agg_v32i8) align 8 %{{.*}}, ptr byval(%struct.agg_v32i8) align 8 %{{.*}})
// Verify that the following are *not* vector-like aggregate types
diff --git a/clang/test/CodeGen/SystemZ/systemz-abi.c b/clang/test/CodeGen/SystemZ/systemz-abi.c
index 7b2c04ece185ff3..6c26498da423313 100644
--- a/clang/test/CodeGen/SystemZ/systemz-abi.c
+++ b/clang/test/CodeGen/SystemZ/systemz-abi.c
@@ -43,7 +43,7 @@ long long pass_longlong(long long arg) { return arg; }
// CHECK-LABEL: define{{.*}} i64 @pass_longlong(i64 %{{.*}})
__int128 pass_int128(__int128 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_int128(ptr noalias sret(i128) align 8 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_int128(ptr noalias sret(i128) align 8 %{{.*}}, ptr byval(i128) align 8 %0)
float pass_float(float arg) { return arg; }
// CHECK-LABEL: define{{.*}} float @pass_float(float %{{.*}})
@@ -52,34 +52,34 @@ double pass_double(double arg) { return arg; }
// CHECK-LABEL: define{{.*}} double @pass_double(double %{{.*}})
long double pass_longdouble(long double arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_longdouble(ptr noalias sret(fp128) align 8 %{{.*}}, ptr %0)
+// CHECK-LABEL: define{{.*}} void @pass_longdouble(ptr noalias sret(fp128) align 8 %{{.*}}, ptr byval(fp128) align 8 %0)
// Complex types
_Complex char pass_complex_char(_Complex char arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_complex_char(ptr noalias sret({ i8, i8 }) align 1 %{{.*}}, ptr %{{.*}}arg)
+// CHECK-LABEL: define{{.*}} void @pass_complex_char(ptr noalias sret({ i8, i8 }) align 1 %{{.*}}, ptr byval({ i8, i8 }) align 1 %{{.*}}arg)
_Complex short pass_complex_short(_Complex short arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_complex_short(ptr noalias sret({ i16, i16 }) align 2 %{{.*}}, ptr %{{.*}}arg)
+// CHECK-LABEL: define{{.*}} void @pass_complex_short(ptr noalias sret({ i16, i16 }) align 2 %{{.*}}, ptr byval({ i16, i16 }) align 2 %{{.*}}arg)
_Complex int pass_complex_int(_Complex int arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_complex_int(ptr noalias sret({ i32, i32 }) align 4 %{{.*}}, ptr %{{.*}}arg)
+// CHECK-LABEL: define{{.*}} void @pass_complex_int(ptr noalias sret({ i32, i32 }) align 4 %{{.*}}, ptr byval({ i32, i32 }) align 4 %{{.*}}arg)
_Complex long pass_complex_long(_Complex long arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_complex_long(ptr noalias sret({ i64, i64 }) align 8 %{{.*}}, ptr %{{.*}}arg)
+// CHECK-LABEL: define{{.*}} void @pass_complex_long(ptr noalias sret({ i64, i64 }) align 8 %{{.*}}, ptr byval({ i64, i64 }) align 8 %{{.*}}arg)
_Complex long long pass_complex_longlong(_Complex long long arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_complex_longlong(ptr noalias sret({ i64, i64 }) align 8 %{{.*}}, ptr %{{.*}}arg)
+// CHECK-LABEL: define{{.*}} void @pass_complex_longlong(ptr noalias sret({ i64, i64 }) align 8 %{{.*}}, ptr byval({ i64, i64 }) align 8 %{{.*}}arg)
_Complex float pass_complex_float(_Complex float arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_complex_float(ptr noalias sret({ float, float }) align 4 %{{.*}}, ptr %{{.*}}arg)
+// CHECK-LABEL: define{{.*}} void @pass_complex_float(ptr noalias sret({ float, float }) align 4 %{{.*}}, ptr byval({ float, float }) align 4 %{{.*}}arg)
_Complex double pass_complex_double(_Complex double arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_complex_double(ptr noalias sret({ double, double }) align 8 %{{.*}}, ptr %{{.*}}arg)
+// CHECK-LABEL: define{{.*}} void @pass_complex_double(ptr noalias sret({ double, double }) align 8 %{{.*}}, ptr byval({ double, double }) align 8 %{{.*}}arg)
_Complex long double pass_complex_longdouble(_Complex long double arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_complex_longdouble(ptr noalias sret({ fp128, fp128 }) align 8 %{{.*}}, ptr %{{.*}}arg)
+// CHECK-LABEL: define{{.*}} void @pass_complex_longdouble(ptr noalias sret({ fp128, fp128 }) align 8 %{{.*}}, ptr byval({ fp128, fp128 }) align 8 %{{.*}}arg)
// Aggregate types
@@ -94,7 +94,7 @@ struct agg_2byte pass_agg_2byte(struct agg_2byte arg) { return arg; }
struct agg_3byte { char a[3]; };
struct agg_3byte pass_agg_3byte(struct agg_3byte arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_agg_3byte(ptr noalias sret(%struct.agg_3byte) align 1 %{{.*}}, ptr %{{.*}})
+// CHECK-LABEL: define{{.*}} void @pass_agg_3byte(ptr noalias sret(%struct.agg_3byte) align 1 %{{.*}}, ptr byval(%struct.agg_3byte) align 1 %{{.*}})
struct agg_4byte { char a[4]; };
struct agg_4byte pass_agg_4byte(struct agg_4byte arg) { return arg; }
@@ -102,15 +102,15 @@ struct agg_4byte pass_agg_4byte(struct agg_4byte arg) { return arg; }
struct agg_5byte { char a[5]; };
struct agg_5byte pass_agg_5byte(struct agg_5byte arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_agg_5byte(ptr noalias sret(%struct.agg_5byte) align 1 %{{.*}}, ptr %{{.*}})
+// CHECK-LABEL: define{{.*}} void @pass_agg_5byte(ptr noalias sret(%struct.agg_5byte) align 1 %{{.*}}, ptr byval(%struct.agg_5byte) align 1 %{{.*}})
struct agg_6byte { char a[6]; };
struct agg_6byte pass_agg_6byte(struct agg_6byte arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_agg_6byte(ptr noalias sret(%struct.agg_6byte) align 1 %{{.*}}, ptr %{{.*}})
+// CHECK-LABEL: define{{.*}} void @pass_agg_6byte(ptr noalias sret(%struct.agg_6byte) align 1 %{{.*}}, ptr byval(%struct.agg_6byte) align 1 %{{.*}})
struct agg_7byte { char a[7]; };
struct agg_7byte pass_agg_7byte(struct agg_7byte arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_agg_7byte(ptr noalias sret(%struct.agg_7byte) align 1 %{{.*}}, ptr %{{.*}})
+// CHECK-LABEL: define{{.*}} void @pass_agg_7byte(ptr noalias sret(%struct.agg_7byte) align 1 %{{.*}}, ptr byval(%struct.agg_7byte) align 1 %{{.*}})
struct agg_8byte { char a[8]; };
struct agg_8byte pass_agg_8byte(struct agg_8byte arg) { return arg; }
@@ -118,7 +118,7 @@ struct agg_8byte pass_agg_8byte(struct agg_8byte arg) { return arg; }
struct agg_16byte { char a[16]; };
struct agg_16byte pass_agg_16byte(struct agg_16byte arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_agg_16byte(ptr noalias sret(%struct.agg_16byte) align 1 %{{.*}}, ptr %{{.*}})
+// CHECK-LABEL: define{{.*}} void @pass_agg_16byte(ptr noalias sret(%struct.agg_16byte) align 1 %{{.*}}, ptr byval(%struct.agg_16byte) align 1 %{{.*}})
// Float-like aggregate types
@@ -135,7 +135,7 @@ struct agg_double pass_agg_double(struct agg_double arg) { return arg; }
struct agg_longdouble { long double a; };
struct agg_longdouble pass_agg_longdouble(struct agg_longdouble arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_agg_longdouble(ptr noalias sret(%struct.agg_longdouble) align 8 %{{.*}}, ptr %{{.*}})
+// CHECK-LABEL: define{{.*}} void @pass_agg_longdouble(ptr noalias sret(%struct.agg_longdouble) align 8 %{{.*}}, ptr byval(%struct.agg_longdouble) align 8 %{{.*}})
struct agg_float_a8 { float a __attribute__((aligned (8))); };
struct agg_float_a8 pass_agg_float_a8(struct agg_float_a8 arg) { return arg; }
@@ -144,7 +144,7 @@ struct agg_float_a8 pass_agg_float_a8(struct agg_float_a8 arg) { return arg; }
struct agg_float_a16 { float a __attribute__((aligned (16))); };
struct agg_float_a16 pass_agg_float_a16(struct agg_float_a16 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_agg_float_a16(ptr noalias sret(...
|
|
This looks correct to me. I'm just wondering if there's any unexpected change of generated code due to this - did you do a comparison before/after of some larger code bases? |
|
byval changes the calling convention: instead of passing a pointer, it copies the pointed-to value into the argument list. It's not clear to me why msan cares about whether a pointer is an implicit or explicit value... but if it does, we'd want to use an attribute that doesn't have an existing ABI impact. |
|
I just checked with a few small examples, and while the ABI does not seem to change, the code generation looks broken for tail recursion: Regarding MSan/DFSan, they need to know whether to copy value's or pointer's shadow into arguments' TLS. MSan already has checks for that ( |
I'd guess the SystemZ backend doesn't implement byval properly (since it's not required for the C calling convention, probably nobody would notice any issues).
I don't see why you'd need to distinguish between two cases that generate identical IR, unless you think the additional information can somehow lead to more accurate diagnostics. Other targets emit struct arguments in a similar way without any special handling in msan. |
|
There is special byval handling in MSan: If a target does not make use of byval, I would argue that this part of MSan is broken there. A quick experiment shows that x86_64 uses byval: As for the reasons why this all is the case, I think this is because of how arguments' shadow TLS is defined: it contains shadows of all argument values, where arguments are understood in terms of C, and not in terms of the ABI. This definition is then used in the runtime. See, for example, the handling of long doubles in Here is wants the shadow of the long double itself, even if the target ABI requires passing it through a pointer. |
|
There's a special case in the msan handling precisely because byval makes the ABI different. (If you look at the x86 code, you'll see that the generated code with byval is significantly different.) I don't think I know enough about dfsan to follow that argument. |
|
In any case:
|
|
I don't quite get why can't we use byval here. How is this different from x86_64? Both s390x and x86_64 ABIs require passing large structs via a pointer, why can x86_64 implement this using byval in LLVM and s390x can't? I agree that currently s390x backend does not handle it properly and the current version of this PR is not suitable for inclusion, but what is the conceptual problem here? |
Wrong. x86_64 passes structs by value (copying the data into the argument list). It just looks like a pointer in the IR because LLVM can't really handle large values in registers. The semantics of byval are "the call instruction allocates memory, and copies the argument into that memory". That generally means copying the memory into the argument list, although LangRef doesn't actually mandate that. If byval were actually implemented correctly for SystemZ, adding would completely change the generated code in a way that's not actually compatible with the SystemZ C ABI. |
|
Sorry, I my wording was not precise enough, it is indeed important that we create a copy, and not pass a pointer to the original. Still, what you described matches the s390x ABI: Ah, that's the source of my confusion. I didn't realize the call instruction had to make a copy, I thought it just had to be done somewhere. "The attribute implies that a hidden copy of the pointee is made between the caller and the callee" actually does mean the former, but one has to squint to see that. The way you phrased it is much clearer. So in the following example: x84_64 generates: and relies on the backend to expand so the creation of the copy is explicit in the LLVM IR. Even though the ABIs are saying roughly the same thing, it's implemented differently. I wonder if it would still be beneficial to switch s390x to byval? I think it can be done in a way that correctly implements the ABI, even though it would of course be more complex than this PR. An obvious benefit is that s390x would become more similar to x86_64, but maybe there are some drawbacks that I'm not seeing. I revisited MSan's I will check if there is a different solution for DFSan. It currently passes the label of the pointer to the copy, which is always 0, instead of the label of the actual value, to vararg functions on s390x. Even though this is similar to what MSan does, the difference is that the DFSan runtime (e.g., |
|
byval generally results in worse code. It has two problems:
x86-64 uses it because it was the simplest way to implement the ABI requirements; llvm.call.preallocated.setup theoretically solves the same problem, but it's a lot more complicated. |
SystemZ ABI mandates that in order to pass large objects by value, one should be place them on stack and pass the callee a pointer. Currently on LLVM IR level it's impossible to distinguish whether a pointer argument was there in the C code, or whether it was introduced in order to satisfy the ABI requirement.
MSan and DFSan need to know that in order to correctly fill the arguments' shadow TLS. Fix by adding ByVal attributes to synthetic pointer arguments. This attribute serves exactly this purpose.
This resolves an outstanding MSan issue, and enables the future DFSan support.