use report_fatal_error() to report errors for missing metadata.

set metadata unless metadata is non-NULL. I have not found a case where metadata is NULL. But nevertheless, let us do this to avoid set a potential NULL metadata.

abandon this one. use git monorepo https://reviews.llvm.org/D65664 since we have llvm/clang inter-dependence here.

abandon this one. use git monorepo https://reviews.llvm.org/D65664 since we have llvm/clang inter-dependence here.

reorder parameter arrayType to make it with default nullptr and simplifies the code.

The clang change https://reviews.llvm.org/D65615.
The BPF backend change https://reviews.llvm.org/D65618.

The corresponding IR intrinsic interface change is at https://reviews.llvm.org/D65617
The BPF backend to utilize this information is at https://reviews.llvm.org/D65618

Abandon this revision for now as one of its variants has been merged.

abandon this one. A revised version with most tests has been merged.

The commit has been merged.

The followup patch for this change is at https://reviews.llvm.org/D64598.

The followup patch for this is at https://reviews.llvm.org/D64606.

@jdoerfert Thanks for commenting on my diff. Will find the appropriate person to review.

change test case so we can test metadata is generated correctly.

@jdoerfert Thanks. I will address your comments and submit standalone test cases for these intrinsics. I do have some BPF unit tests using these intrinsics. But I agree that some standalone unit test cases are also needed.

@jdoerfert Thanks for comments. I will submit another patch to add adequate tests for clang frontend as you suggested.

change some tests where spFlags will not be used in order for the codes easier backport to older compiler (llvm8).

@eli.friedman has reviewed a related patch (https://reviews.llvm.org/D61809) and is okay with the whole approach. @ast is also okay with the patch, so I will start to land soon. Thanks!

@rsmith Ping again, do you have any comments on this patch? Thanks!

for bpf maps section, change from strict .maps section name to .maps* section name, that is, the map section name starts with .maps

fixed a bug in BTFDebug related to btf fwd type generation

@rsmith Just ping, could you take a look at the clang change? If possible, could you share your opinion? Thanks!

handle unnamed bitfield properly. these struct/union members are not encoded in debuginfo, so skip them during preserve_*_access_index IR intrinsics generation.

change void *__builtin_preserve_access_index(void *) to const void * _builtin_preserve_access_index(const void *).

reword the lang doc for builtin_preserve_access_index() to be more user focused
treat using builtin_preserve_access_index() without -g or in nested way as compiler errors

@rsmith I have proposed one clang intrinsic and three IR intrinsics for CO-RE. Could you take a look and share your opinions as well? Thanks!

@eli.friedman Hi, Just ping. I have removed getParents() for ASTContext and added description of the new intrinsic in language doc. Could you take a look? Thanks!

Handling the following case:

@eli.friedman I removed the usage of astcontext getParents() stuff. Instead, I mark the region upfront. I also added the intrinsic __builtin_preserve_access_index() into clang
lang extention doc. Could you take a look at new patch? Thanks!

supported new .maps format by avoiding aggressive pointee type pruning for those map definition structures.
(https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=abd29c9314595b1ee5ec6c61d7c49a497ffb30a3)

do not use ctx.getParents() to check whether a GEP candidate inside a preserve_access_index. instead, mark the region upfront. Add the new clang intrinsic into the language doc.

@eli.friedman Sorry for replying late. I am outside US and currently in PTO. Will back to US soon to address your comments.

@rsmith @eli.friedman Ping again. Do you have any comments on my proposed clang/IR intrinsics? Thanks!

remove bpf offsetreloc option and use FileCheck in the test.

remove BPF "checkoffsetreloc" subtarget option as just honoring what user says. Later on, if needed, we can introduce a pass to help show user what is transformed and what is not so user can decide whether there is any missing __builtin_preserve_access_index()'s.

@rsmith @eli.friedman Do you have any comments on the clang intrinsic interface in this patch and the llvm intrinsics interface at https://reviews.llvm.org/D61810?

@eli.friedman @rsmith @lebedev.ri I just loaded a new set of patches. Notable changes include

• introduce clang intrinsic base = __builtin_preserve_access_index(base), which is used to identify relocatable geps. This is a buy-in approach from user so we do not pay performance penalty for transforming gep's to intrinsics and later back to gep with some performance loss.
• for IR intrinsics preserve_*_access_index, struct/union names are removed from the argument list. Instead a new metadata type MD_preserve_access_index is introduced. This metadata has struct/union debuginfo type and attached to preserve_*_access_index.

removed typename from intrinsics, using metadata (ditype) instead

add clang builtin __builtin_preserve_access_index

I did some analysis on the performance side, specifically, # of instructions generated.
My example is cilium at https://github.com/cilium/cilium/tree/master/bpf, bpf program bpf_lxc.c.

This update of three patches mostly a few intrinsic format changes (array +num_of_zeros for gep, array/struct -inst_name), and correspond adjustments to fix bugs.
This will unblock other kernel bpf developers which can work in parallel in kernel side.

changes due to intrinsics change

change new intrinsic format, add num_of_zeros for array, remove inst_name

array index may not be constant and remove inst_name

@eli.friedman thanks for the review. I just did some experiments with some performance sensitive bpf programs. It looks the approach I am taking here cause worse case 30% instructions generated by llvm at -O2 level.
So the current approach to convert to intrinsics blindly and later on convert back unnecessary intrinsics to regular GEP and necessary intrinsics to relocatable GEP won't work.
The main reason for performance degradation is:

• intrinsics causing less optimal codes which may contributes 5% performance loss for a relative small routines, for a large routines, I have see the loss is more than 20%.
• intrinsics impact inline decision and less inlining has negative impacts on the performance.

@rsmith @eli.friedman Hi, I just implemented a new version which generates intrinsics only. Three intrinsics are implemented for array/union/structure separately as their handling and parameters are different. Could you take a look? Thanks!

@lebedev.ri For comments "What about other targets, does this need some default expansion?", currently these intrinsics only used by bpf and lowered to GEP in an IR pass implemented by bpf backend. I can certainly implement some default expansion if it is desirable. Is the SelectionDAG the proper place to implement the default lowering?

use the new preserve_*_access_index intrinsics

split intrinsics into three, and add langdef.

do not generate gep any more, only intrinsics

@rsmith @eli.friedman Thanks for your comments. I fully agree that it seems awkward that we have both GEP and intrinsic generation. I will try to do some experiments here to only have intrinsic generation. My only concern is possible performance degradation. I will post here once I got concrete implementation and numbers.

getAsInstruction probably doesn't do what you want here. It actually creates a new Instruction*, instead of casting the expression. Probably this is the source of the verifier error.

@eli.friedman I just uploaded a new revision which uses a newly-added intrinsics to pass information from clang generated IR to BPF IR pass. Could you take a look and give your opinion on this approach?
This patch is not mergeable as I have a llvm verification failure described in my previous comments. If you have some insight on this issue, please let me know. Thanks!

My current implementation has an issue like below: