fix a typo (BPFPreserveType.cpp => BPFPreserveDIType.cpp) in the comments.

change llvm.bpf_pdit to llvm.btf_type_id variable name to make it easy to understand.

The corresponding clang side of change is https://reviews.llvm.org/D74668

The corresponding LLVM side of change is https://reviews.llvm.org/D74572

make this diff llvm BPF backend only. The clang side change will be posted separately.

Addressing some Alexei's comments:

Change comments in clang CGBuiltin.cpp to make it easier to understand.
Remove BPFMIPreserveDIType.cpp and fold into existing BPFMISimplifyPatchable.cpp.
Other Simplifications.

add the type_id assignment to FieldInfo relocation .btf.ext ELF section.

Please double check that all selftests/bpf still pass with and without alu32 before landing.

arm64 uses this approach, but arm64 doesn't have 8-bit subregisters and it works ?

make isTruncateFree more like AArch64 one. Remove !getHasAlu32() from isTruncateFree where it is not needed. But !getHasAlu32() is still needed in isZExtFree. Otherwise, we will get a llvm assertion like:

  CLNG-LLC [test_maps] test_xdp_noinline.o
llc: ../lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:978: void {anonymous}::SelectionDAGLegalize::LegalizeOp(llvm::SDNode*): Assertion `(TLI.getTypeAction(*DAG.getContext(), Op.getValueType()) == TargetLowering::TypeLegal || Op.getOpcode() == ISD::TargetConstant || Op.getOpcode() == ISD::Register) && "Unexpected illegal type!"' failed.
Stack dump:
0.      Program arguments: llc -march=bpf -mcpu=v2 -filetype=obj -o /data/users/yhs/work/net-next/tools/testing/selftests/bpf/no_alu32/test_xdp_noinline.$

Note that I cannot do

In isTruncateFree() do
return NumBits1 > NumBits2;

This will permit a type truncate operation e.g., from 64bit to 8bit. This will cause a llvm internal assertion later since llvm expects
it produces a backend supported legal subregister type , which is 32bit in this case. PowerPC has similar implementation

bool PPCTargetLowering::isTruncateFree(Type *Ty1, Type *Ty2) const {
  if (!Ty1->isIntegerTy() || !Ty2->isIntegerTy())
    return false;
  unsigned NumBits1 = Ty1->getPrimitiveSizeInBits();
  unsigned NumBits2 = Ty2->getPrimitiveSizeInBits();
  return NumBits1 == 64 && NumBits2 == 32;
}

add support isZExtFree as well

@jrfastab I added isTruncateFree() callback in BPF IR Lowering (isel) phase. See https://reviews.llvm.org/D74101. This should help with the case where packet data/data_end is truncated with 32bit MOV.

address Andrii's comments. Always use the typedef type name if it is available.

revert back to previous version where no hacking with machine IR optimization to add extra "<<" and "s>>". Will try to enhance verifier to do the work.

@tstellar Thanks for your comments. The MachineInstr SSA based optimization is really a ugly hack. Currently verifier is not able to verify the program correctly, i.e., rejects the program for certain patterns related subregister uses. The commit message provides more information. @ast suggested to look at whether we can improve kernel verifier and I will take a look there. If kernel can be enhanced, I will drop the second MachineInstr SSA pass.

Added a machineinstr SSA target phase to add left/aright shift operations if the 32bit subregister is used for sle/slt/sge/sgt operations.

fix potential overflow issues

Indeed tried kernel selftests, revert the previous workaround like

diff --git a/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c b/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c
index d22e438198cf..608a06871572 100644
--- a/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c
+++ b/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c
@@ -44,10 +44,7 @@ int sysctl_tcp_mem(struct bpf_sysctl *ctx)
        unsigned long tcp_mem[TCP_MEM_LOOPS] = {};
        char value[MAX_VALUE_STR_LEN];
        unsigned char i, off = 0;
-       /* a workaround to prevent compiler from generating
-        * codes verifier cannot handle yet.
-        */
-       volatile int ret;
+       int ret;

The current kernel did not handle signed compare well for 32bit registers. I will need to make that change first in order to make this llvm change effective.

To handle cases (exposed when running linux kernel samples/bpf/sockex1) like below:

unsigned long long load_byte(void *skb, unsigned long long off) asm("llvm.bpf.load.byte");
unsigned long long test(void *skb) {
  return load_byte(skb, 10);
}

previous approach emits load_byte as load_byte function is in module function reference list.
But actually this function is not called so we should not emit extern func BTF for this.

use the same XADD opcode for new instructions.

implement exchange instruction

add a test case for extern function with arguments.

@dblaikie The root cause has been identified by @rnk . A clang plugin is used which requires the following patch:

https://github.com/llvm/llvm-project/commit/5128026467cbc17bfc796d94bc8e40e52a9b0752

which has been merged. The original extern variable debug info type has been relanded. So we are good. Thanks!

Could you link to particular bots that have logs showing they ran this test? (perhaps the logs have been retired by now, though - since this patch was reverted :/ - but then at least seeing which bots run BPF tests would be helpful)

@rnk Great to know the reason. Thanks!

@rnk I somehow cannot reproduce the issue locally. If you can let me know how to exactly reproduce the issue, I might be able to also help with some kind of solution, although frankly speaking, I am not a clang expert.

Removed REQUIRES: bpf-registered-target. This is not needed any more if using %clang_cc1. Tested by build clang with and without BPF target.

@rnk I just submitted a patch https://reviews.llvm.org/D71818 to use %clang_cc1 instead of %clang in the test. Could you check whether this fixed your issue or not?

I cannot remove BPF target requirement as only BPF target can trigger debug info generation for extern variables. This is by design as discussed in D70696 due to concerns it may blow up debug info binary size for existing applications. This may be relaxed in the future through some flags.
BTW, as far as I know, BPF is enabled for a lot of bots, ubuntu, x86, ppc, arm, etc. So the coverage of this future should be fine.

use bpf_type.info lower two bits to encode func scope info

merged.

remove some redundant test cases

@Jac1494 Based on early discussion, we cannot just issue debuginfo for externs when -g is specified as this may cause debuginfo (and final binary) size bloat. One possible way is to enable it when -fstandalone-debug is specified. @dblaikie can you comment on this patch and general possible forward path for extern variable debug info support?

extern variables can have attribute((weak)) attribute. Support it as well.

@dblaikie I consolidated test cases as you suggested. Along the way, I also added some more tests. I already answered some of your questions earlier. Please let me know what you think.

consolidated into less test files, added more test cases along the way.

In D70696#1767616, @dblaikie wrote:

Many of the test cases could be collapsed into one file - using different variables that are used, unused, locally or globally declared, etc.

clang-format change (from @aprantl) , add a little details into summary related to recent discussion,
move tests from CodeGenCXX to CodeGen as they are all C tests.

@dblaikie The concern for that users using -fstandalone-debug may suddenly see a bloat of external var debug info is totally valid. Let me just stick to BPF use case for now. If somebody else ever wants this information with -fstandalone-debug, the restriction can be evaluated and relaxed then.

@dblaikie Good points. I will guard external variable debug info generation under -fstandalone-debug flag.

@probinson for the question,

Does bpf require debug info for the declaration of global_var in noref.c ?

No, bpf only cares the referenced external global variables. So my current implementation does not emit debug info
for external global_var in noref.c.

@SouraVX Currently, I filtered all external globals if they are not used. See

for (auto D: ExternalDeclarations) {
  if (!D || D->isInvalidDecl() || D->getPreviousDecl() || !D->isUsed())
    continue;

additional test case is added to ensure debuginfo for externs is indeed generated in BTF.

The backend supports extern variable debug info as well. The following restrictions in the old code is removed.

 -    // Ignore external globals for now.
-    if (!Global.hasInitializer() && Global.hasExternalLinkage())
-      continue;

@dblaikie As suggested, just submitted two separate patches:

• https://reviews.llvm.org/D70696 for clang/llvm change to generate debuginfo for extern variables
• https://reviews.llvm.org/D70697 for BPF backend to consume such debuginfo.