User Details
- User Since
- Oct 4 2018, 11:50 PM (233 w, 2 d)
Mon, Mar 13
Sun, Mar 12
Added some comments to explain the rationale behind CheckPointer and SeePointer. If it looks okay, could you accept the patch?
update the comment.
add comments to explain CheckPointer and SeePointer.
Fri, Mar 10
Sun, Feb 26
Jan 21 2023
@dyung I just pushed the fix to the 'main' branch (https://github.com/llvm/llvm-project/commit/183d075055c591dedead7ece972f1bdea611aa3b). Please check it out. Thanks for reporting!
Jan 20 2023
Jan 19 2023
Sorry, I double checked. '-fstack-protector -fno-stack-protector' will not result in warnings. So the patch LGTM. So gentoo people can add -fno-stack-protector to suppress warnings.
@ast With this patch, gentoo clang compilation will hit the warning even if people appends -fno-stack-protector. Is this okay? In general, if the option is '-fstack-protector -fno-stack-protector', we should not issue warning, right?
Jan 18 2023
@compnerd could you also take a look at this patch?
Jan 17 2023
LGTM. Thanks!
Jan 7 2023
Jan 6 2023
LGTM. I will add // REQUIRES: bpf-registered-target in the test to ensure it only executed with bpf target.
Jan 3 2023
Please separate the patch to clang and llvm part.
Dec 29 2022
Dec 28 2022
Somehow, CCACHE does not work for me, which has an error message like missing 'malloc/malloc.h'. I used the following scripts without CCACHE.
$ cat build_llvm_debug.sh cd llvm-project/llvm/build
Otherwise, the patch LGTM.
Dec 27 2022
Nov 23 2022
Thanks for the code simplification. LGTM.
Nov 1 2022
- improve the bug description in release note.
Oct 31 2022
@aaron.ballman or @dblaikie I have addressed the comments, could you take a look again?
Oct 28 2022
Oct 27 2022
- Add the bug fix description in Release Note
- Actually compare debuginfo btf_type_tag output instead of marking 'expected-no-diagnostics'.
Yes, will add the bug fix brief description to clang/docs/ReleaseNotes.rst
Oct 26 2022
Oct 17 2022
@aprantl The following example shows the difference between gcc and clang w.r.t. reserved names.
Thanks. The change looks good to me from BPF perspective as it fixed the regression in https://github.com/llvm/llvm-project/issues/57872.
Sep 28 2022
- simplify test case, add more CHECK's and add comments to explain the test.
Sep 27 2022
Sep 21 2022
Sep 20 2022
Hi, we found a regression with some bpf code with this patch. The following shows the problem:
Sep 9 2022
I hacked the kernel x64 jit part, with the following jit for sext load, it seems working okay from jit perspective.
+/* LDX: dst_reg = *(s8*)(src_reg + off) */ +static void emit_ldx_sext(u8 **pprog, u32 size, u32 dst_reg, u32 src_reg, int off) +{ + u8 *prog = *pprog; + + switch (size) { + case BPF_B: + /* Emit 'movsx rax, byte ptr [rax + off]' */ + EMIT3(add_2mod(0x48, src_reg, dst_reg), 0x0F, 0xBE); + break; + case BPF_H: + /* Emit 'movsx rax, word ptr [rax + off]' */ + EMIT3(add_2mod(0x48, src_reg, dst_reg), 0x0F, 0xBF); + break; + case BPF_W: + /* Emit 'movsx rax, dword ptr [rax+0x14]' */ + EMIT2(add_2mod(0x48, src_reg, dst_reg), 0x63); + break; + } + emit_insn_suffix(&prog, src_reg, dst_reg, off); + *pprog = prog; +}
All major architectures support sign extension load (x64, arm64, sparc, webassembly, lanai, etc.)
Sep 7 2022
Aug 18 2022
add two llvm/test/CodeGen/BPF tests.
but if the function is always_inline it can have more than 5 arguments, right?
Kernel changes were assuming at most 2
The compiler only counted the total number of registers won't exceed 5. You could have 5 struct arguments (each one register), or 2 struct arguments (with two registers each) and one integer register.
Whether to pass by value or by reference is a ABI issue, not related to the number of arguments. Checking whether arguments are enough or not is left to BPF backend lowering.
This is an example,
[$~/tmp3] cat t3.c int foo(int a1, int a2, int a3, int a4, int a5, int a6) { return a1 + a2 + a3 + a4 + a5 + a6; } [$ ~/tmp3] clang -target bpf -O2 -S -emit-llvm t3.c [$~/tmp3] clang -target bpf -O2 -S t3.c t3.c:1:5: error: defined with too many args int foo(int a1, int a2, int a3, int a4, int a5, int a6) { ^ 1 error generated. [$ ~/tmp3]
Aug 16 2022
Aug 10 2022
Aug 9 2022
Aug 8 2022
Jul 13 2022
Jun 29 2022
Jun 6 2022
Jun 2 2022
LGTM. But add the reference (https://reviews.llvm.org/D126838) to the commit message to indicate which 'earlier change'. Also let us merge it unti kernel side change is in reasonable shape.
May 24 2022
@compnerd provided a better fix https://reviews.llvm.org/D126093, so I am going to close this one.
Sorry. I missed this patch somehow and did another patch https://reviews.llvm.org/D126223. My patch skipped TL.setNameLoc(DS.getTypeSpecTypeNameLoc()); which probably not correct. Accept this patch and abandon my patch D126223.
May 23 2022
May 20 2022
@compnerd Thanks for reporting. I will take a look as soon as possible.
May 12 2022
- also set signed enum for dwarf::DW_ATE_signed_char int.