[PATCH] D132144: [Clang][BPF] Support record argument with direct values

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

In D132144#3732685 <https://reviews.llvm.org/D132144#3732685>, @anakryiko wrote:

> Are there any restrictions about number of struct arguments that can be passed in? Kernel changes were assuming at most 2, should we have a test that tests passing 3 structs that fit into 5 input registers and another test that passes 3 structs that do not fit in 5 input registers? And what should be the behavior in the latter case -- compilation error or switch to "pass by reference"?

This is a good question. I actually tried to add such a test but unfortunately didn't find a good way to do it.
The following is an example,

  [$ ~/tmp3] cat t2.c
  struct t1 {
    long a;
  };
  struct t2 {
    long a;
    long b;
  };
  
  long foo1(long a1, long a2, long a3, long a4, long a5) {
    return a1 + a2 + a3 + a4 + a5;
  }
  
  long foo2(struct t1 a1, long a2, long a3, long a4, long a5) {
    return a1.a + a2 + a3 + a4 + a5;
  }
  
  long foo3(struct t2 a1, long a2, long a3, long a4, long a5) {
    return a1.a + a2 + a3 + a4 + a5;
  }
  [$ ~/tmp3] clang -target bpf -O2 -emit-llvm -S -Xclang -disable-llvm-passes t2.c
  [$ ~/tmp3] clang -target bpf -O2 -emit-llvm -S t2.c
  [$ ~/tmp3] clang -target bpf -O2 -S t2.c
  t2.c:17:6: error: defined with too many args
  long foo3(struct t2 a1, long a2, long a3, long a4, long a5) {
       ^
  1 error generated.
  [$~/tmp3]

You can see compiler does not flag the error during IR generation and IR optimization.
The error is actually flagged out during IR->Machine IR translation in file

  Target/BPF/BPFISelLowering.cpp:      fail(DL, DAG, "defined with too many args");

Because the error is trigged by the BPF backend, which means that .s or .o file will
not get generated so we could not really test it. This is the case for all other
backend triggered fail or fatal_error.

But I think I will add a llvm test from IR to asm code which will include struct arguments (16 bytes)
and it will extend to use 5 registers for all arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132144