-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[BPF] add allows-misaligned-mem-access target feature #167013
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
Most backends use a subtarget feature to indicate whether unaligned access is allowed. See AArch64, AMDGPU, ARM, LoongArch, etc. Besides consistency, this is also significantly more convenient for frontends, especially those that produce bitcode directly (e.g. Zig), and less problematic for library users too as it avoids global state. On a more general note (not directed at you @clairechingching!): I don't know what the community consensus is on this (if there is one), but IMHO, backends severely over(ab)use |
|
Hey @yonghong-song I tried moving the |
|
misaligned memory access is bad for performance and may have issues for verification (or make verification more complex). Do you have concrete C code to illustrate this? Can the C code easily converted to aligned memory access? cc @4ast |
|
@yonghong-song The kernel verifier is indeed very restrictive and for good reason. This feature is intended for user-space eBPF, where the decision to allow misaligned access is up to the implementer. In such environments, allowing misaligned accesses is far more performant as it drastically reduces the number of instructions required to perform common memory operations. By making it optional, we leave kernel BPF behavior unchanged while allowing the implementer to improve user space performance when the platform supports it. |
Just to back up this point: |
d58f2eb to
6bbc701
Compare
|
Apologies for the rebase noise! I realized I initially submitted the PR against the release branch I'm targeting and not main. I've just rebased on main! |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
In linux kernel, BPF_F_ANY_ALIGNMENT flag indicates the verifier will handle misalignment. This is not on by default and need user space to enable that during program load. I did a hack below in kernel to enable BPF_F_ANY_ALIGNMENT by default like below: With the above kernel hack, I enabled the unaligned memory access by default like below With the combination of the above llvm and kernel changes, I tried and still some selftest failures. Some kernel changes will be needed or compiler needs to be more selective allows misaligned memory access. Since the kernel already supports misalignment, I off-line discussed with @4ast and we think maybe it is a good idea to have kernel misalignment support on by default since kernel has support for BPF_F_ANY_ALIGNMENT already. In that sense, llvm support for misalignment sounds a good idea. Please check BTF.td file which already has a few feature examples. |
what tests failed without kernel hack? |
A bunch of failures like below: or etc.
Agree. |
6bbc701 to
50825fb
Compare
|
@4ast @yonghong-song @alexrp hey I've moved it to subtarget as that seems to be the preference, but I haven't made allowing misalignment the default. I'm happy to do that but below tests will fail because hardcode specific alignments etc Failed Tests (10): |
50825fb to
4f9f73b
Compare
|
Let us have allowing misaligned memory access off by default now and your pull request already did this. I will need to sort out in kernel side. If kernel side eventually allows misaligned memory access by default, at this point, we can flip to have llvm misaligned memory access on by default. BTW, there is a format issue flagged by llvm test. Please fix. |
|
For failed tests you mentioned above with misaligned access by default, we can add flag to disable misaligned access for this tests. This will be future work. |
4f9f73b to
d80b163
Compare
|
My previous experiments with llvm mis-alignment support plus kernel change actually does not solve verification failure. (Maybe I accidentally used the wrong compiler). For one example, in kernel/bpf/verifier.c, we have Note that for case PTR_TO_STACK, where 'strict' is set to be true to enforce aligned memory access. If the llvm generates mis-aligned stack access, verification will fail. Need more investigation. |
This enables misaligned memory access when the feature is enabled
d80b163 to
a4a645f
Compare
|
all feedback addressed |
|
@clairechingching Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
This proposal adds
allows-misaligned-mem-accesstarget feature to BPF target that lets users enable allowing misaligned memory accesses.The motivation behind the proposal is user space eBPF VMs (interpreters or JITs running in user space) typically run on real CPUs where unaligned memory accesses are acceptable (or handled efficiently) and can be enabled to simplify lowering and improve performance. In contrast, kernel eBPF must obey verifier constraints and platform-specific alignment restrictions.
A new CLI option keeps kernel behavior unchanged while giving userspace VMs an explicit opt-in to enable more permissive codegen. It supports both use-cases without diverging codebases.