- User Since
- May 8 2022, 6:30 AM (72 w, 9 h)
Rebase, will merge this in after CI run.
Thu, Sep 21
Wed, Sep 20
Thank you for all the feedback!
I've applied the latest suggestions and going to merge this revision once CI is done.
Changes in accordance with @MaskRay feedback.
Tue, Sep 19
Could you please take a look at this revision?
I think I addressed all feedback.
Fri, Sep 15
Fix for failed unit test pragma-attribute-supported-attributes-list.test
Thu, Sep 14
- Use (new) attribute((preserve_static_offset)) instead of attribute((btf_decl_tag("ctx"))).
- Rename files, passes, etc accordingly.
Tue, Sep 5
Missing dot at the end of the comment.
Mon, Sep 4
Unit tests are passing on S390 under emulation w/o issues.
Thank you for the feedback, I think I covered all but one items.
The last remaining item is to verify that unit test is working on the bigendian system.
I'll update the revision once I check that.
After retesting kernel build with LLVM=1 and libbpf patch to reconstruct btf_decl_tag , statistics for BPF selftests looks as follows:
- out of 653 object files 13 have some differences with and w/o this change;
- for 2 programs there is small instruction count increase (+2 insn total);
- for 5 programs there is small instruction decrease (-6 insn total);
- 6 programs differ slightly but number of instructions is the same.
Wed, Aug 30
Tue, Aug 29
Mon, Aug 28
Sun, Aug 27
- Removed special Sema processing and btf_decl_tag("ctx") prioritization in CGExpr, preserve access index calls are now handled by BPFContextMarker transformation.
- Correctness fixes for isPointerOperand().
Aug 24 2023
Aug 23 2023
Fixes basing on feedback from @MaskRay.
Fix for incorrect BTF::CommonType::getVlen() mask.
Aug 21 2023
Rebase, want to see green CI build
If you have some time, could you please take a look at my response?
If you have some time, could you please take a look?
Aug 18 2023
By the way, it is still possible to test this using e.g. llc or clang. As noted previously in this thread the default error handler for llc/clang is stateful, so it proceeds the code generation, produces assembly code and returns non-zero error code.
Aug 17 2023
Hi Yonghong, could you please take a look?
As described here, this fixes one of the selftest compilation bugs that occur when LLVM_ENABLE_EXPENSIVE_CHECKS is on. Sorry for the long description, I tried to explain why instruction selection would always cover cases that BPFMIPeepholeTruncElim covered.
Aug 16 2023
As an additional data point, the same example but w/o section specification compiles fine:
After this revision landed yesterday one BPF kernel selftest (written in C) stopped building. I narrowed the issue to the following example:
Aug 14 2023
Aug 13 2023
The build bot failure (here) was caused by newly added test case field-reloc-st-imm.ll. When LLVM is built with LLVM_ENABLE_EXPENSIVE_CHECKS=ON flag this test triggered bug described in D157806. I rebased this revision on top of D157806. Could you please double check if you are still ok with these changes?
I have finished changes for this revision, please take a look.
Rebased on top of D157806.
It is possible to avoid separate versions for CORE_ST32 and CORE_ST64, merging these simplifies implementation for D140804.
Hold-on, I need to update this one to be more in line with changes necessary for D140804 functionality.
Aug 10 2023
Build bot reports a failure (link):
Could you please take a look at my response?
Aug 6 2023
Changes according to Yonghong's comments
Aug 4 2023
Note: CI failures seem to be completely unrelated...
Aug 3 2023
I messed up.
@ast is right, I missed the fact that clang already exits with return code 1 when any of the conditions reported by fail(...) happen, e.g.:
Aug 2 2023
After examining source code for llvm-lit, I found an option --crash for not. Here is the portable incantation:
I grepped the code base and see that finalizeLowering() logic is customized only in a handful of cases:
In my opinion, whether or not to repeat MI->getOperand(NumDefs) is a matter of personal preference in this case. Current code is ok and this change does not change any related functionality. I suggest to withheld from this change to avoid cluttering commit history.
This error could be detected using the following test case:
Aug 1 2023
Thank you for the detailed response!
Jul 31 2023
Jul 30 2023
Jul 28 2023
I left a few minor comments.
All-in-all I think these modifications are fine if it makes BPF backend simpler to work with.
However, I'm not sure if fixing my comments is worthwhile if Alexei objects the whole idea.
Jul 27 2023
if you have a minute, could you please take a look?
I used this locally and it proved to be convenient for debugging (e.g. here).
Could you please take a look at my reply?
Jul 25 2023
To better reiterate my position on the warning for ExternalSymbolSDNode: in C analogy such warnings would be generated by the linker, not compiler.
So, I vote to agree with @tamird here and remove the warning (or at-least make it optional).
Jul 24 2023
This reproduces in the aya integration tests in this PR on this commit: https://github.com/aya-rs/bpf-linker/pull/77/commits/6624b7d673a7b65cc634e94a4a8a63f79aa90f64
It's not exactly a simple repro. I can try to give you just the IR if that'd be helpful?
Jul 23 2023
Please note that a number of unit tests fails with this patch applied:
- LLVM :: CodeGen/BPF/many_args1.ll
- LLVM :: CodeGen/BPF/many_args2.ll
- LLVM :: CodeGen/BPF/struct_ret1.ll
- LLVM :: CodeGen/BPF/vararg1.ll
- LLVM :: CodeGen/BPF/warn-call.ll
Jul 20 2023
I tried adding a test similar to assemble-disassemble.ll:
Jul 16 2023
Could you please take a look at this revision?
It adds support for CO-RE relocations printing in llvm-objdump.
While current implementation works, there are two aspects that might be sub-optimal:
- ELF relocations are not unified with BTF relocations and thus there are two functions: llvm-objdump.cpp:printRelocation(), llvm-objdump.cpp:printBTFRelocation();
- The BTFParser instance is not shared between LLVMSymbolizer (used for source line printing) and llvm-objdump.cpp:disassembleObject() (used for relocations printing).
Jul 13 2023
I opened D155176 with packed attribute shuffling, will commit after pre-merge checks are complete for it.
(Want to make sure that the change does not cause issues with MSVC).
Jul 12 2023
Sorry, I was away from the PC. Thank you for committing the fix, I will update the unit test to execute in both big and little endian.
However, it looks like there is one more issue, as reported here for address sanitizer:
Changes requested by @MaskRay in the last review round.