luismarques created this revision.
luismarques added reviewers: asb, mundaym.
Herald added subscribers: vkmr, frasercrmck, evandro, apazos, sameer.abuasal, 
s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, 
rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, 
sabuasal, simoncook, johnrusso, rbar.
luismarques requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Should we add the `byval` parameter attribute to struct parameters?

Pros:

- Improves optimizations;
- Used by MSan instrumentation pass (without it a test will fail).

Cons:

- ?

An example of an optimization without `byval` and with `byval`:

  $ cat test.c
  struct S { char c[32]; };
  void foo(struct S s) {
      s.c[0] = 42;
  }
  $ clang --target=riscv64-linux -O2 -S -o- test.c

Without `byval`:

  foo:
          addi    a1, zero, 42
          sb      a1, 0(a0)
          ret

With `byval`:

  foo:
          ret

I'm not sure what the impact would be of also adding the attribute to vector 
parameters, so this patch doesn't do so.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97896

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/RISCV/riscv32-ilp32-abi.c
  clang/test/CodeGen/RISCV/riscv32-ilp32-ilp32f-abi.c
  clang/test/CodeGen/RISCV/riscv32-ilp32-ilp32f-ilp32d-abi.c
  clang/test/CodeGen/RISCV/riscv32-ilp32d-abi.c
  clang/test/CodeGen/RISCV/riscv32-ilp32f-abi.c
  clang/test/CodeGen/RISCV/riscv32-ilp32f-ilp32d-abi.c
  clang/test/CodeGen/RISCV/riscv64-lp64-lp64f-lp64d-abi.c
  clang/test/CodeGen/RISCV/riscv64-lp64d-abi.c

Index: clang/test/CodeGen/RISCV/riscv64-lp64d-abi.c
===================================================================
--- clang/test/CodeGen/RISCV/riscv64-lp64d-abi.c
+++ clang/test/CodeGen/RISCV/riscv64-lp64d-abi.c
@@ -240,7 +240,7 @@
 
 struct int_double_int_s { int a; double b; int c; };
 
-// CHECK: define{{.*}} void @f_int_double_int_s_arg(%struct.int_double_int_s* %a)
+// CHECK: define{{.*}} void @f_int_double_int_s_arg(%struct.int_double_int_s* byval(%struct.int_double_int_s) align 8 %a)
 void f_int_double_int_s_arg(struct int_double_int_s a) {}
 
 // CHECK: define{{.*}} void @f_ret_int_double_int_s(%struct.int_double_int_s* noalias sret(%struct.int_double_int_s) align 8 %agg.result)
Index: clang/test/CodeGen/RISCV/riscv64-lp64-lp64f-lp64d-abi.c
===================================================================
--- clang/test/CodeGen/RISCV/riscv64-lp64-lp64f-lp64d-abi.c
+++ clang/test/CodeGen/RISCV/riscv64-lp64-lp64f-lp64d-abi.c
@@ -159,7 +159,7 @@
   int64_t a, b, c, d;
 };
 
-// CHECK-LABEL: define{{.*}} void @f_agg_large(%struct.large* %x)
+// CHECK-LABEL: define{{.*}} void @f_agg_large(%struct.large* byval(%struct.large) align 8 %x)
 void f_agg_large(struct large x) {
   x.a = x.b + x.c + x.d;
 }
@@ -186,7 +186,7 @@
 // Scalars passed on the stack should not have signext/zeroext attributes
 // (they are anyext).
 
-// CHECK-LABEL: define{{.*}} signext i32 @f_scalar_stack_1(i64 %a.coerce, [2 x i64] %b.coerce, i128 %c.coerce, %struct.large* %d, i8 zeroext %e, i8 signext %f, i8 %g, i8 %h)
+// CHECK-LABEL: define{{.*}} signext i32 @f_scalar_stack_1(i64 %a.coerce, [2 x i64] %b.coerce, i128 %c.coerce, %struct.large* byval(%struct.large) align 8 %d, i8 zeroext %e, i8 signext %f, i8 %g, i8 %h)
 int f_scalar_stack_1(struct tiny a, struct small b, struct small_aligned c,
                      struct large d, uint8_t e, int8_t f, uint8_t g, int8_t h) {
   return g + h;
@@ -217,7 +217,7 @@
 
 // CHECK-LABEL: define{{.*}} void @f_va_caller()
 void f_va_caller() {
-  // CHECK: call signext i32 (i32, ...) @f_va_callee(i32 signext 1, i32 signext 2, i64 3, double 4.000000e+00, double 5.000000e+00, i64 {{%.*}}, [2 x i64] {{%.*}}, i128 {{%.*}}, %struct.large* {{%.*}})
+  // CHECK: call signext i32 (i32, ...) @f_va_callee(i32 signext 1, i32 signext 2, i64 3, double 4.000000e+00, double 5.000000e+00, i64 {{%.*}}, [2 x i64] {{%.*}}, i128 {{%.*}}, %struct.large* byval(%struct.large) align 8 {{%.*}})
   f_va_callee(1, 2, 3LL, 4.0f, 5.0, (struct tiny){6, 7, 8, 9},
               (struct small){10, NULL}, (struct small_aligned){11},
               (struct large){12, 13, 14, 15});
Index: clang/test/CodeGen/RISCV/riscv32-ilp32f-ilp32d-abi.c
===================================================================
--- clang/test/CodeGen/RISCV/riscv32-ilp32f-ilp32d-abi.c
+++ clang/test/CodeGen/RISCV/riscv32-ilp32f-ilp32d-abi.c
@@ -109,7 +109,7 @@
   return (struct float_int32_s){1.0, 2};
 }
 
-// CHECK: define{{.*}} void @f_float_int64_s_arg(%struct.float_int64_s* %a)
+// CHECK: define{{.*}} void @f_float_int64_s_arg(%struct.float_int64_s* byval(%struct.float_int64_s) align 8 %a)
 void f_float_int64_s_arg(struct float_int64_s a) {}
 
 // CHECK: define{{.*}} void @f_ret_float_int64_s(%struct.float_int64_s* noalias sret(%struct.float_int64_s) align 8 %agg.result)
@@ -233,7 +233,7 @@
 
 struct int_float_int_s { int a; float b; int c; };
 
-// CHECK: define{{.*}} void @f_int_float_int_s_arg(%struct.int_float_int_s* %a)
+// CHECK: define{{.*}} void @f_int_float_int_s_arg(%struct.int_float_int_s* byval(%struct.int_float_int_s) align 4 %a)
 void f_int_float_int_s_arg(struct int_float_int_s a) {}
 
 // CHECK: define{{.*}} void @f_ret_int_float_int_s(%struct.int_float_int_s* noalias sret(%struct.int_float_int_s) align 4 %agg.result)
@@ -243,7 +243,7 @@
 
 struct int64_float_s { int64_t a; float b; };
 
-// CHECK: define{{.*}} void @f_int64_float_s_arg(%struct.int64_float_s* %a)
+// CHECK: define{{.*}} void @f_int64_float_s_arg(%struct.int64_float_s* byval(%struct.int64_float_s) align 8 %a)
 void f_int64_float_s_arg(struct int64_float_s a) {}
 
 // CHECK: define{{.*}} void @f_ret_int64_float_s(%struct.int64_float_s* noalias sret(%struct.int64_float_s) align 8 %agg.result)
Index: clang/test/CodeGen/RISCV/riscv32-ilp32f-abi.c
===================================================================
--- clang/test/CodeGen/RISCV/riscv32-ilp32f-abi.c
+++ clang/test/CodeGen/RISCV/riscv32-ilp32f-abi.c
@@ -23,7 +23,7 @@
 
 struct double_double_s { double d; double e; };
 
-// CHECK: define{{.*}} void @f_double_double_s_arg(%struct.double_double_s* %a)
+// CHECK: define{{.*}} void @f_double_double_s_arg(%struct.double_double_s* byval(%struct.double_double_s) align 8 %a)
 void f_double_double_s_arg(struct double_double_s a) {}
 
 // CHECK: define{{.*}} void @f_ret_double_double_s(%struct.double_double_s* noalias sret(%struct.double_double_s) align 8 %agg.result)
@@ -35,7 +35,7 @@
 
 struct int_double_s { int a; double b; };
 
-// CHECK: define{{.*}} void @f_int_double_s_arg(%struct.int_double_s* %a)
+// CHECK: define{{.*}} void @f_int_double_s_arg(%struct.int_double_s* byval(%struct.int_double_s) align 8 %a)
 void f_int_double_s_arg(struct int_double_s a) {}
 
 // CHECK: define{{.*}} void @f_ret_int_double_s(%struct.int_double_s* noalias sret(%struct.int_double_s) align 8 %agg.result)
Index: clang/test/CodeGen/RISCV/riscv32-ilp32d-abi.c
===================================================================
--- clang/test/CodeGen/RISCV/riscv32-ilp32d-abi.c
+++ clang/test/CodeGen/RISCV/riscv32-ilp32d-abi.c
@@ -77,7 +77,7 @@
   return (struct double_float_s){1.0, 2.0};
 }
 
-// CHECK: define{{.*}} void @f_double_double_s_arg_insufficient_fprs(float %a, double %b, double %c, double %d, double %e, double %f, double %g, %struct.double_double_s* %h)
+// CHECK: define{{.*}} void @f_double_double_s_arg_insufficient_fprs(float %a, double %b, double %c, double %d, double %e, double %f, double %g, %struct.double_double_s* byval(%struct.double_double_s) align 8 %h)
 void f_double_double_s_arg_insufficient_fprs(float a, double b, double c, double d,
     double e, double f, double g, struct double_double_s h) {}
 
@@ -116,7 +116,7 @@
   return (struct double_int32_s){1.0, 2};
 }
 
-// CHECK: define{{.*}} void @f_double_int64_s_arg(%struct.double_int64_s* %a)
+// CHECK: define{{.*}} void @f_double_int64_s_arg(%struct.double_int64_s* byval(%struct.double_int64_s) align 8 %a)
 void f_double_int64_s_arg(struct double_int64_s a) {}
 
 // CHECK: define{{.*}} void @f_ret_double_int64_s(%struct.double_int64_s* noalias sret(%struct.double_int64_s) align 8 %agg.result)
@@ -143,11 +143,11 @@
   return (struct double_int8_zbf_s){1.0, 2};
 }
 
-// CHECK: define{{.*}} void @f_double_int8_s_arg_insufficient_gprs(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e, i32 %f, i32 %g, i32 %h, %struct.double_int8_s* %i)
+// CHECK: define{{.*}} void @f_double_int8_s_arg_insufficient_gprs(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e, i32 %f, i32 %g, i32 %h, %struct.double_int8_s* byval(%struct.double_int8_s) align 8 %i)
 void f_double_int8_s_arg_insufficient_gprs(int a, int b, int c, int d, int e,
                                           int f, int g, int h, struct double_int8_s i) {}
 
-// CHECK: define{{.*}} void @f_struct_double_int8_insufficient_fprs(float %a, double %b, double %c, double %d, double %e, double %f, double %g, double %h, %struct.double_int8_s* %i)
+// CHECK: define{{.*}} void @f_struct_double_int8_insufficient_fprs(float %a, double %b, double %c, double %d, double %e, double %f, double %g, double %h, %struct.double_int8_s* byval(%struct.double_int8_s) align 8 %i)
 void f_struct_double_int8_insufficient_fprs(float a, double b, double c, double d,
                                            double e, double f, double g, double h, struct double_int8_s i) {}
 
@@ -240,7 +240,7 @@
 
 struct int_double_int_s { int a; double b; int c; };
 
-// CHECK: define{{.*}} void @f_int_double_int_s_arg(%struct.int_double_int_s* %a)
+// CHECK: define{{.*}} void @f_int_double_int_s_arg(%struct.int_double_int_s* byval(%struct.int_double_int_s) align 8 %a)
 void f_int_double_int_s_arg(struct int_double_int_s a) {}
 
 // CHECK: define{{.*}} void @f_ret_int_double_int_s(%struct.int_double_int_s* noalias sret(%struct.int_double_int_s) align 8 %agg.result)
@@ -250,7 +250,7 @@
 
 struct int64_double_s { int64_t a; double b; };
 
-// CHECK: define{{.*}} void @f_int64_double_s_arg(%struct.int64_double_s* %a)
+// CHECK: define{{.*}} void @f_int64_double_s_arg(%struct.int64_double_s* byval(%struct.int64_double_s) align 8 %a)
 void f_int64_double_s_arg(struct int64_double_s a) {}
 
 // CHECK: define{{.*}} void @f_ret_int64_double_s(%struct.int64_double_s* noalias sret(%struct.int64_double_s) align 8 %agg.result)
@@ -260,7 +260,7 @@
 
 struct char_char_double_s { char a; char b; double c; };
 
-// CHECK-LABEL: define{{.*}} void @f_char_char_double_s_arg(%struct.char_char_double_s* %a)
+// CHECK-LABEL: define{{.*}} void @f_char_char_double_s_arg(%struct.char_char_double_s* byval(%struct.char_char_double_s) align 8 %a)
 void f_char_char_double_s_arg(struct char_char_double_s a) {}
 
 // CHECK: define{{.*}} void @f_ret_char_char_double_s(%struct.char_char_double_s* noalias sret(%struct.char_char_double_s) align 8 %agg.result)
Index: clang/test/CodeGen/RISCV/riscv32-ilp32-ilp32f-ilp32d-abi.c
===================================================================
--- clang/test/CodeGen/RISCV/riscv32-ilp32-ilp32f-ilp32d-abi.c
+++ clang/test/CodeGen/RISCV/riscv32-ilp32-ilp32f-ilp32d-abi.c
@@ -170,7 +170,7 @@
   int32_t a, b, c, d;
 };
 
-// CHECK-LABEL: define{{.*}} void @f_agg_large(%struct.large* %x)
+// CHECK-LABEL: define{{.*}} void @f_agg_large(%struct.large* byval(%struct.large) align 4 %x)
 void f_agg_large(struct large x) {
   x.a = x.b + x.c + x.d;
 }
@@ -197,7 +197,7 @@
 // Scalars passed on the stack should not have signext/zeroext attributes
 // (they are anyext).
 
-// CHECK-LABEL: define{{.*}} i32 @f_scalar_stack_1(i32 %a.coerce, [2 x i32] %b.coerce, i64 %c.coerce, %struct.large* %d, i8 zeroext %e, i8 signext %f, i8 %g, i8 %h)
+// CHECK-LABEL: define{{.*}} i32 @f_scalar_stack_1(i32 %a.coerce, [2 x i32] %b.coerce, i64 %c.coerce, %struct.large* byval(%struct.large) align 4 %d, i8 zeroext %e, i8 signext %f, i8 %g, i8 %h)
 int f_scalar_stack_1(struct tiny a, struct small b, struct small_aligned c,
                      struct large d, uint8_t e, int8_t f, uint8_t g, int8_t h) {
   return g + h;
@@ -226,7 +226,7 @@
 void f_scalar_stack_5(double a, int64_t b, double c, int64_t d, int e,
                       int64_t f, float g, double h, long double i) {}
 
-// CHECK-LABEL: define{{.*}} void @f_agg_stack(double %a, i64 %b, double %c, i64 %d, i32 %e.coerce, [2 x i32] %f.coerce, i64 %g.coerce, %struct.large* %h)
+// CHECK-LABEL: define{{.*}} void @f_agg_stack(double %a, i64 %b, double %c, i64 %d, i32 %e.coerce, [2 x i32] %f.coerce, i64 %g.coerce, %struct.large* byval(%struct.large) align 4 %h)
 void f_agg_stack(double a, int64_t b, double c, int64_t d, struct tiny e,
                  struct small f, struct small_aligned g, struct large h) {}
 
@@ -237,7 +237,7 @@
 int f_va_callee(int, ...);
 
 // CHECK-LABEL: define{{.*}} void @f_va_caller()
-// CHECK: call i32 (i32, ...) @f_va_callee(i32 1, i32 2, i64 3, double 4.000000e+00, double 5.000000e+00, i32 {{%.*}}, [2 x i32] {{%.*}}, i64 {{%.*}}, %struct.large* {{%.*}})
+// CHECK: call i32 (i32, ...) @f_va_callee(i32 1, i32 2, i64 3, double 4.000000e+00, double 5.000000e+00, i32 {{%.*}}, [2 x i32] {{%.*}}, i64 {{%.*}}, %struct.large* byval(%struct.large) align 4 {{%.*}})
 void f_va_caller() {
   f_va_callee(1, 2, 3LL, 4.0f, 5.0, (struct tiny){6, 7, 8, 9},
               (struct small){10, NULL}, (struct small_aligned){11},
Index: clang/test/CodeGen/RISCV/riscv32-ilp32-ilp32f-abi.c
===================================================================
--- clang/test/CodeGen/RISCV/riscv32-ilp32-ilp32f-abi.c
+++ clang/test/CodeGen/RISCV/riscv32-ilp32-ilp32f-abi.c
@@ -50,6 +50,6 @@
 void f_scalar_stack_3(double a, int64_t b, double c, int64_t d, int e,
                       int64_t f, int32_t g, double h, long double i) {}
 
-// CHECK-LABEL: define{{.*}} void @f_agg_stack(double %a, i64 %b, double %c, i64 %d, i32 %e.coerce, [2 x i32] %f.coerce, i64 %g.coerce, %struct.large* %h)
+// CHECK-LABEL: define{{.*}} void @f_agg_stack(double %a, i64 %b, double %c, i64 %d, i32 %e.coerce, [2 x i32] %f.coerce, i64 %g.coerce, %struct.large* byval(%struct.large) align 4 %h)
 void f_agg_stack(double a, int64_t b, double c, int64_t d, struct tiny e,
                  struct small f, struct small_aligned g, struct large h) {}
Index: clang/test/CodeGen/RISCV/riscv32-ilp32-abi.c
===================================================================
--- clang/test/CodeGen/RISCV/riscv32-ilp32-abi.c
+++ clang/test/CodeGen/RISCV/riscv32-ilp32-abi.c
@@ -48,6 +48,6 @@
 void f_scalar_stack_3(double a, int64_t b, double c, int64_t d, int e,
                       int64_t f, float g, double h, long double i) {}
 
-// CHECK-LABEL: define{{.*}} void @f_agg_stack(double %a, i64 %b, double %c, i64 %d, i32 %e.coerce, [2 x i32] %f.coerce, i64 %g.coerce, %struct.large* %h)
+// CHECK-LABEL: define{{.*}} void @f_agg_stack(double %a, i64 %b, double %c, i64 %d, i32 %e.coerce, [2 x i32] %f.coerce, i64 %g.coerce, %struct.large* byval(%struct.large) align 4 %h)
 void f_agg_stack(double a, int64_t b, double c, int64_t d, struct tiny e,
                  struct small f, struct small_aligned g, struct large h) {}
Index: clang/lib/CodeGen/TargetInfo.cpp
===================================================================
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -10728,7 +10728,7 @@
           llvm::IntegerType::get(getVMContext(), XLen), 2));
     }
   }
-  return getNaturalAlignIndirect(Ty, /*ByVal=*/false);
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/!Ty->isVectorType());
 }
 
 ABIArgInfo RISCVABIInfo::classifyReturnType(QualType RetTy) const {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D97896: [Clang][RISC... Luís Marques via Phabricator via cfe-commits

Reply via email to