[clang][BPF] support type exist/size and enum exist/value relocations

[yonghong-song]

Summary

This patch added the following additional compile-once
run-everywhere (CO-RE) relocations:

• existence/size of typedef, struct/union or enum type
• enum value and enum value existence

These additional relocations will make CO-RE bpf programs more
adaptive for potential kernel internal data structure changes.

For existence/size relocations, the following two code patterns
are supported:

1. __builtin_preserve_type_info(*(<type> *)0, flag);
2. <type> var; __builtin_preserve_field_info(var, flag);

flag = 0 for existence relocation and flag = 1 for size relocation.

For enum value existence and enum value relocations, the following code
pattern is supported:

uint64_t __builtin_preserve_enum_value(*(<enum_type> *)<enum_value>,
                                       flag);

flag = 0 means existence relocation and flag = 1 for enum value.
relocation. In the above <enum_type> can be an enum type or
a typedef to enum type. The <enum_value> needs to be an enumerator
value from the same enum type. The return type is uint64_t to
permit potential 64bit enumerator values.

[anakryiko]

Awesome, that's exactly what we need for BPF helper availability checks!

Can you please also add test that this pattern works:

return __builtin_preserve_field_info((btf_bpf_read_branch_records)0, FIELD_EXISTENCE);

Also for non-func pointer typedefs, something like this would work as well, right?

return __builtin_preserve_field_info(*(T *)0, FIELD_EXISTENCE);

[jrfastab]

Agreed also useful for us. I can pull it in and test if that is helpful.

[yonghong-song]

Sounds good. Will try to add support for union/struct type as well.

[yonghong-song]

clang change only and added tests

[yonghong-song]

The llvm change is here: https://reviews.llvm.org/D83878

[ast]

lgtm.
curious what happens when type is defined within args, like:
__builtin_preserve_field_info(*(struct { int a; } *)0, 2);

[anakryiko]

This is awesome. I'll apply patches locally and will play with them tonight to see if everything works in libbpf as well. Thanks!

[yonghong-song]

In D83242#2154145, @ast wrote:

lgtm.
curious what happens when type is defined within args, like:
__builtin_preserve_field_info(*(struct { int a; } *)0, 2);

-bash-4.4$ clang -target bpf -O2 -g -S t1.c
t1.c:3:40: error: __builtin_preserve_field_info argument 1 invalid

return __builtin_preserve_field_info(*(struct { int a; } *)0, 2);

it will flag out an error since an non-empty name is required
as we will need it to generate relocation.

if (!RT || RT->getDecl()->getDeclName().isEmpty())

With an name, it works properly.

-bash-4.4$ cat t1.c

unsigned unit1() {
  return __builtin_preserve_field_info(*(struct t { int a; } *)0, 2);
}
-bash-4.4$ clang -target bpf -O2 -g -S t1.c                                                        
-bash-4.4$

[yonghong-song]

fix clang-format warnings.

[anakryiko]

After thinking about this a bit more, I think adding a new TYPE_EXISTENCE (instead of FIELD_EXISTENCE) relocation type is the better way to handle this. It would allow Clang to be stricter as to what is passed into FIELD_EXISTENCE (only fields, not types) vs TYPE_EXISTENCE(only types, no fields). It will make it cleaner on libbpf side as well, because the validation and relocation logic is different between handling fields and types. So overall, I think it will be beneficial on every level of the stack to separate them. I hope it's not too hard to make this change?

[yonghong-song]

In D83242#2167721, @anakryiko wrote:

After thinking about this a bit more, I think adding a new TYPE_EXISTENCE (instead of FIELD_EXISTENCE) relocation type is the better way to handle this. It would allow Clang to be stricter as to what is passed into FIELD_EXISTENCE (only fields, not types) vs TYPE_EXISTENCE(only types, no fields). It will make it cleaner on libbpf side as well, because the validation and relocation logic is different between handling fields and types. So overall, I think it will be beneficial on every level of the stack to separate them. I hope it's not too hard to make this change?

Yes. Sounds better to separate them as they handle disjoint patterns. Will make the change.

[yonghong-song]

use a different reloc type TYPE_EXISTENCE (defined in llvm/lib/Target/BPF/BPFCORE.h) instead of old FIELD_EXISTENCE. This will simplify libbpf processing.

[yonghong-song]

@anakryiko Please rebase on top of latest clang. There is a recent core change which touches BPF code which I am also modifying.

[yonghong-song]

support relocations for type_sizeof and enum_value as well.

[yonghong-song]

One potential missing relocation is to test whether a particular enum value (from an enum type) exists or not.

One way is to create a new relocation type ENUM_VALUE_EXISTENCE where "access_string" to encode the enum value. This is different from TYPE_EXISTENCE for enum type where the "access_string" is "0".

Alternatively, we could have TYPE_EXISTENCE to include enum value existence as well. "access_string" is say "0" for typedef/record/enum types and "0:<enum_value>" for enum value.

WDYT?

[yonghong-song]

use a different __builtin function for new relocations to they are easily discoverable in user space and this also make implementation more structured.

[anakryiko]

looks great, I'll complete libbpf support for all these ASAP

[yonghong-song]

enum value relocation change: use ld_imm64 so we support 64bit enum value from day one. put enumerator name as the AccessString.

[anakryiko]

LGTM. One question: why didn't we run into the need for SeqNumVal trick with field-based relocations? We seem to need it for all other types (including type ID-based), but not for field-based?

[yonghong-song]

In D83242#2192130, @anakryiko wrote:

LGTM. One question: why didn't we run into the need for SeqNumVal trick with field-based relocations? We seem to need it for all other types (including type ID-based), but not for field-based?

good question. In general CSE can only happen if
the base address is the same, for a->b->c->d and a->b->c, portion of address calculation may be shared (CSE'ed).
If the base address is different (e.g., as different variable), CSE cannot happen since the base address is different.

The CSE with different types can happen if the base address is nullptr. This may happen when you try to test
whether a member exists or not. For example:

-bash-4.4$ cat t.c
struct t { int a; int b; };
typedef struct t t;
int test(struct t *arg1, t *arg2) {

return __builtin_preserve_field_info(((struct t *)0)->b, 1) +
       __builtin_preserve_field_info(((__t *)0)->b, 1);

}
-bash-4.4$

Although clang generates two IR builtins tagged with "struct t" and "t" debuginfo type respectively, the IR type for the first argument base is the same as "struct t" as typedef is ignored at IR level. So in this case, two builtin_preserve_field_info will be CSE'ed to one. I think this is okay, we are trying to test whether a field is available, the correct result is still returned.

This is different than testing whether a type exists or not where we want to precisely test whether that type exists or not, so we want to preserve typedef type vs. struct t type.

[anakryiko]

This is different than testing whether a type exists or not where we want to precisely test whether that type exists or not, so we want to preserve typedef type vs. struct t type.

typedef t and struct t are also different for field relocations (those t is in different namespaces, actually), so it might be a good idea to disambiguate those for field relocations as well?

[yonghong-song]

In D83242#2193989, @anakryiko wrote:

This is different than testing whether a type exists or not where we want to precisely test whether that type exists or not, so we want to preserve typedef type vs. struct t type.

typedef t and struct t are also different for field relocations (those t is in different namespaces, actually), so it might be a good idea to disambiguate those for field relocations as well?

Just to be clear. The type recorded in relocation will be always valid since we have type_id there. For field exists, due to we may use nullptr as the object, if CSE happens, it is possible the root type id may be the "struct t" instead of "typedef t" (or the other ordering) assuming the definition "typedef struct t t;". But from root type_id, libbpf will be able to correctly trace down to the field and do the relocation properly.

On the other hand, I agree that this is not ideal as we may use a different root type than what user specified in the source. But everything is hidden and invisible to user so I won't have user visible impact.

To fix this in clang is actually tricky, we do not want to add seq number to every preserve_field_info* IR builtin since this is not really necessary for non-existence relocations. Also preserve_field_info* although used by bpf only so far is llvm generic builtin, I would hesitate to change it with new seq_num fields. The best will be have another bpf specific builtin
for field existence, but given the current mechanism also works, I probably will keep it this way.

[anakryiko]

In D83242#2195456, @yonghong-song wrote:

In D83242#2193989, @anakryiko wrote:

This is different than testing whether a type exists or not where we want to precisely test whether that type exists or not, so we want to preserve typedef type vs. struct t type.

typedef t and struct t are also different for field relocations (those t is in different namespaces, actually), so it might be a good idea to disambiguate those for field relocations as well?

Just to be clear. The type recorded in relocation will be always valid since we have type_id there. For field exists, due to we may use nullptr as the object, if CSE happens, it is possible the root type id may be the "struct t" instead of "typedef t" (or the other ordering) assuming the definition "typedef struct t t;". But from root type_id, libbpf will be able to correctly trace down to the field and do the relocation properly.

On the other hand, I agree that this is not ideal as we may use a different root type than what user specified in the source. But everything is hidden and invisible to user so I won't have user visible impact.

Hm.. This root type is used by libbpf to find candidate in kernel BTF. Name and kind of that type is what is used. When you have typedef t and struct t, their names match, but their kinds don't. So it could lead to inability to find a candidate.

To fix this in clang is actually tricky, we do not want to add seq number to every preserve_field_info* IR builtin since this is not really necessary for non-existence relocations. Also preserve_field_info* although used by bpf only so far is llvm generic builtin, I would hesitate to change it with new seq_num fields. The best will be have another bpf specific builtin
for field existence, but given the current mechanism also works, I probably will keep it this way.

A new built-in is definitely a big hassle. It hasn't been a problem so far and the possibility is really slim. If it's a problem to do this for existing built-in, let's leave it as is for now. Just something to keep in mind.

[yonghong-song]

Hm.. This root type is used by libbpf to find candidate in kernel BTF. Name and kind of that type is what is used. When you have typedef t and struct t, their names match, but their kinds don't. So it could lead to inability to find a candidate.

The inproper CSE can only happen if the declaration in user program like struct t { ... }; typedef struct t t; and user try to find existence of both struct t and typedef t. We could have issue if kernel does not have struct t { ...} and only has typedef t. Theoretically this could happen, but as you mentioned the chance is really slim. So will not change current implementation right now.

clang/include/clang/Basic/BuiltinsBPF.def

 #endif
 
 // Get record field information.
 TARGET_BUILTIN(__builtin_preserve_field_info, "Ui.", "t", "")
 
 // Get BTF type id.
 TARGET_BUILTIN(__builtin_btf_type_id, "Ui.", "t", "")
 
+// Get type information.
+TARGET_BUILTIN(__builtin_preserve_type_info, "Ui.", "t", "")
+
+// Preserve enum value.
+TARGET_BUILTIN(__builtin_preserve_enum_value, "Li.", "t", "")
+
 #undef BUILTIN
 #undef TARGET_BUILTIN

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_import_on_definition : Warning<
   InGroup<IgnoredAttributes>;
 
 def err_preserve_field_info_not_field : Error<
   "__builtin_preserve_field_info argument %0 not a field access">;
 def err_preserve_field_info_not_const: Error<
   "__builtin_preserve_field_info argument %0 not a constant">;
 def err_btf_type_id_not_const: Error<
   "__builtin_btf_type_id argument %0 not a constant">;
+def err_preserve_type_info_invalid : Error<
+  "__builtin_preserve_type_info argument %0 invalid">;
+def err_preserve_type_info_not_const: Error<
+  "__builtin_preserve_type_info argument %0 not a constant">;
+def err_preserve_enum_value_invalid : Error<
+  "__builtin_preserve_enum_value argument %0 invalid">;
+def err_preserve_enum_value_not_const: Error<
+  "__builtin_preserve_enum_value argument %0 not a constant">;
 
 def err_bit_cast_non_trivially_copyable : Error<
   "__builtin_bit_cast %select{source|destination}0 type must be trivially copyable">;
 def err_bit_cast_type_size_mismatch : Error<
   "__builtin_bit_cast source size does not equal destination size (%0 vs %1)">;
 
 // SYCL-specific diagnostics
 def warn_sycl_kernel_num_of_template_params : Warning<

clang/lib/CodeGen/CGBuiltin.cpp

case NEON::BI__builtin_neon_vuqaddq_v: {
     return EmitNeonCall(CGM.getIntrinsic(Int, Ty), Ops, "vuqadd");
   }
   }
 }
 
 Value *CodeGenFunction::EmitBPFBuiltinExpr(unsigned BuiltinID,
                                            const CallExpr *E) {
   assert((BuiltinID == BPF::BI__builtin_preserve_field_info ||
-          BuiltinID == BPF::BI__builtin_btf_type_id) &&
+          BuiltinID == BPF::BI__builtin_btf_type_id ||
+          BuiltinID == BPF::BI__builtin_preserve_type_info ||
+          BuiltinID == BPF::BI__builtin_preserve_enum_value) &&
          "unexpected BPF builtin");
 
+  // A sequence number, injected into IR builtin functions, to
+  // prevent CSE given the only difference of the funciton
+  // may just be the debuginfo metadata.
+  static uint32_t BuiltinSeqNum;
+
   switch (BuiltinID) {
   default:
     llvm_unreachable("Unexpected BPF builtin");
   case BPF::BI__builtin_preserve_field_info: {
     const Expr *Arg = E->getArg(0);
     bool IsBitField = Arg->IgnoreParens()->getObjectKind() == OK_BitField;
 
     if (!getDebugInfo()) {
case BPF::BI__builtin_btf_type_id: {
     Constant *CV = ConstantInt::get(IntTy, IsLValue);
     llvm::Function *FnBtfTypeId = llvm::Intrinsic::getDeclaration(
         &CGM.getModule(), llvm::Intrinsic::bpf_btf_type_id,
         {FieldVal->getType(), CV->getType()});
     CallInst *Fn = Builder.CreateCall(FnBtfTypeId, {FieldVal, CV, FlagValue});
     Fn->setMetadata(LLVMContext::MD_preserve_access_index, DbgInfo);
     return Fn;
   }
+  case BPF::BI__builtin_preserve_type_info: {
+    if (!getDebugInfo()) {
+      CGM.Error(E->getExprLoc(), "using builtin function without -g");
+      return nullptr;
+    }
+
+    const Expr *Arg0 = E->getArg(0);
+    llvm::DIType *DbgInfo = getDebugInfo()->getOrCreateStandaloneType(
+        Arg0->getType(), Arg0->getExprLoc());
+
+    ConstantInt *Flag = cast<ConstantInt>(EmitScalarExpr(E->getArg(1)));
+    Value *FlagValue = ConstantInt::get(Int64Ty, Flag->getSExtValue());
+    Value *SeqNumVal = ConstantInt::get(Int32Ty, BuiltinSeqNum++);
+
+    llvm::Function *FnPreserveTypeInfo = llvm::Intrinsic::getDeclaration(
+        &CGM.getModule(), llvm::Intrinsic::bpf_preserve_type_info, {});
+    CallInst *Fn =
+        Builder.CreateCall(FnPreserveTypeInfo, {SeqNumVal, FlagValue});
+    Fn->setMetadata(LLVMContext::MD_preserve_access_index, DbgInfo);
+    return Fn;
+  }
+  case BPF::BI__builtin_preserve_enum_value: {
+    if (!getDebugInfo()) {
+      CGM.Error(E->getExprLoc(), "using builtin function without -g");
+      return nullptr;
+    }
+
+    const Expr *Arg0 = E->getArg(0);
+    llvm::DIType *DbgInfo = getDebugInfo()->getOrCreateStandaloneType(
+        Arg0->getType(), Arg0->getExprLoc());
+
+    // Find enumerator
+    const auto *UO = cast<UnaryOperator>(Arg0->IgnoreParens());
+    const auto *CE = cast<CStyleCastExpr>(UO->getSubExpr());
+    const auto *DR = cast<DeclRefExpr>(CE->getSubExpr());
+    const auto *Enumerator = cast<EnumConstantDecl>(DR->getDecl());
+
+    auto &InitVal = Enumerator->getInitVal();
+    std::string InitValStr;
+    if (InitVal.isNegative() || InitVal > uint64_t(INT64_MAX))
+      InitValStr = std::to_string(InitVal.getSExtValue());
+    else
+      InitValStr = std::to_string(InitVal.getZExtValue());
+    std::string EnumStr = Enumerator->getNameAsString() + ":" + InitValStr;
+    Value *EnumStrVal = Builder.CreateGlobalStringPtr(EnumStr);
+
+    ConstantInt *Flag = cast<ConstantInt>(EmitScalarExpr(E->getArg(1)));
+    Value *FlagValue = ConstantInt::get(Int64Ty, Flag->getSExtValue());
+    Value *SeqNumVal = ConstantInt::get(Int32Ty, BuiltinSeqNum++);
+
+    llvm::Function *IntrinsicFn = llvm::Intrinsic::getDeclaration(
+        &CGM.getModule(), llvm::Intrinsic::bpf_preserve_enum_value, {});
+    CallInst *Fn =
+        Builder.CreateCall(IntrinsicFn, {SeqNumVal, EnumStrVal, FlagValue});
+    Fn->setMetadata(LLVMContext::MD_preserve_access_index, DbgInfo);
+    return Fn;
+  }
   }
 }
 
 llvm::Value *CodeGenFunction::
 BuildVector(ArrayRef<llvm::Value*> Ops) {
   assert((Ops.size() & (Ops.size() - 1)) == 0 &&
          "Not a power-of-two sized vector!");
   bool AllConstants = true;

clang/lib/Sema/SemaChecking.cpp

bool Sema::CheckAArch64BuiltinFunctionCall(const TargetInfo &TI,
   case AArch64::BI__builtin_arm_dsb:
   case AArch64::BI__builtin_arm_isb: l = 0; u = 15; break;
   case AArch64::BI__builtin_arm_tcancel: l = 0; u = 65535; break;
   }
 
   return SemaBuiltinConstantArgRange(TheCall, i, l, u + l);
 }
 
+static bool isValidBPFPreserveFieldInfoArg(Expr *Arg) {
+  if (Arg->getType()->getAsPlaceholderType())
+    return false;
+
+  // The first argument needs to be a record field access.
+  // If it is an array element access, we delay decision
+  // to BPF backend to check whether the access is a
+  // field access or not.
+  return (Arg->IgnoreParens()->getObjectKind() == OK_BitField ||
+          dyn_cast<MemberExpr>(Arg->IgnoreParens()) ||
+          dyn_cast<ArraySubscriptExpr>(Arg->IgnoreParens()));
+}
+
+static bool isValidBPFPreserveTypeInfoArg(Expr *Arg) {
+  QualType ArgType = Arg->getType();
+  if (ArgType->getAsPlaceholderType())
+    return false;
+
+  // for TYPE_EXISTENCE/TYPE_SIZEOF reloc type
+  // format:
+  //   1. __builtin_preserve_type_info(*(<type> *)0, flag);
+  //   2. <type> var;
+  //      __builtin_preserve_type_info(var, flag);
+  if (!dyn_cast<DeclRefExpr>(Arg->IgnoreParens()) &&
+      !dyn_cast<UnaryOperator>(Arg->IgnoreParens()))
+    return false;
+
+  // Typedef type.
+  if (ArgType->getAs<TypedefType>())
+    return true;
+
+  // Record type or Enum type.
+  const Type *Ty = ArgType->getUnqualifiedDesugaredType();
+  if (const auto *RT = Ty->getAs<RecordType>()) {
+    if (!RT->getDecl()->getDeclName().isEmpty())
+      return true;
+  } else if (const auto *ET = Ty->getAs<EnumType>()) {
+    if (!ET->getDecl()->getDeclName().isEmpty())
+      return true;
+  }
+
+  return false;
+}
+
+static bool isValidBPFPreserveEnumValueArg(Expr *Arg) {
+  QualType ArgType = Arg->getType();
+  if (ArgType->getAsPlaceholderType())
+    return false;
+
+  // for ENUM_VALUE_EXISTENCE/ENUM_VALUE reloc type
+  // format:
+  //   __builtin_preserve_enum_value(*(<enum_type> *)<enum_value>,
+  //                                 flag);
+  const auto *UO = dyn_cast<UnaryOperator>(Arg->IgnoreParens());
+  if (!UO)
+    return false;
+
+  const auto *CE = dyn_cast<CStyleCastExpr>(UO->getSubExpr());
+  if (!CE || CE->getCastKind() != CK_IntegralToPointer)
+    return false;
+
+  // The integer must be from an EnumConstantDecl.
+  const auto *DR = dyn_cast<DeclRefExpr>(CE->getSubExpr());
+  if (!DR)
+    return false;
+
+  const EnumConstantDecl *Enumerator =
+      dyn_cast<EnumConstantDecl>(DR->getDecl());
+  if (!Enumerator)
+    return false;
+
+  // The type must be EnumType.
+  const Type *Ty = ArgType->getUnqualifiedDesugaredType();
+  const auto *ET = Ty->getAs<EnumType>();
+  if (!ET)
+    return false;
+
+  // The enum value must be supported.
+  for (auto *EDI : ET->getDecl()->enumerators()) {
+    if (EDI == Enumerator)
+      return true;
+  }
+
+  return false;
+}
+
 bool Sema::CheckBPFBuiltinFunctionCall(unsigned BuiltinID,
                                        CallExpr *TheCall) {
   assert((BuiltinID == BPF::BI__builtin_preserve_field_info ||
-          BuiltinID == BPF::BI__builtin_btf_type_id) &&
-         "unexpected ARM builtin");
+          BuiltinID == BPF::BI__builtin_btf_type_id ||
+          BuiltinID == BPF::BI__builtin_preserve_type_info ||
+          BuiltinID == BPF::BI__builtin_preserve_enum_value) &&
+         "unexpected BPF builtin");
 
   if (checkArgCount(*this, TheCall, 2))
     return true;
 
-  Expr *Arg;
-  if (BuiltinID == BPF::BI__builtin_btf_type_id) {
-    // The second argument needs to be a constant int
-    Arg = TheCall->getArg(1);
-    if (!Arg->isIntegerConstantExpr(Context)) {
-      Diag(Arg->getBeginLoc(), diag::err_btf_type_id_not_const)
-          << 2 << Arg->getSourceRange();
-      return true;
-    }
+  Expr *Arg = TheCall->getArg(1);
+  Optional<llvm::APSInt> Value = Arg->getIntegerConstantExpr(Context);
+  diag::kind kind;
+  if (!Value) {
+    if (BuiltinID == BPF::BI__builtin_preserve_field_info)
+      kind = diag::err_preserve_field_info_not_const;
+    else if (BuiltinID == BPF::BI__builtin_btf_type_id)
+      kind = diag::err_btf_type_id_not_const;
+    else if (BuiltinID == BPF::BI__builtin_preserve_type_info)
+      kind = diag::err_preserve_type_info_not_const;
+    else
+      kind = diag::err_preserve_enum_value_not_const;
+    Diag(Arg->getBeginLoc(), kind) << 2 << Arg->getSourceRange();
 
-    TheCall->setType(Context.UnsignedIntTy);
-    return false;
-  }
-
-  // The first argument needs to be a record field access.
-  // If it is an array element access, we delay decision
-  // to BPF backend to check whether the access is a
-  // field access or not.
+  // The first argument
   Arg = TheCall->getArg(0);
-  if (Arg->getType()->getAsPlaceholderType() ||
-      (Arg->IgnoreParens()->getObjectKind() != OK_BitField &&
-       !dyn_cast<MemberExpr>(Arg->IgnoreParens()) &&
-       !dyn_cast<ArraySubscriptExpr>(Arg->IgnoreParens()))) {
-    Diag(Arg->getBeginLoc(), diag::err_preserve_field_info_not_field)
-        << 1 << Arg->getSourceRange();
-    return true;
+  bool InvalidArg = false;
+  bool ReturnUnsignedInt = true;
+  if (BuiltinID == BPF::BI__builtin_preserve_field_info) {
+    if (!isValidBPFPreserveFieldInfoArg(Arg)) {
+      InvalidArg = true;
+      kind = diag::err_preserve_field_info_not_field;
+    }
+  } else if (BuiltinID == BPF::BI__builtin_preserve_type_info) {
+    if (!isValidBPFPreserveTypeInfoArg(Arg)) {
+      InvalidArg = true;
+      kind = diag::err_preserve_type_info_invalid;
+    }
+  } else if (BuiltinID == BPF::BI__builtin_preserve_enum_value) {
+    if (!isValidBPFPreserveEnumValueArg(Arg)) {
+      InvalidArg = true;
+      kind = diag::err_preserve_enum_value_invalid;
+    }
+    ReturnUnsignedInt = false;
   }
 
-  // The second argument needs to be a constant int
-  Arg = TheCall->getArg(1);
-  if (!Arg->isIntegerConstantExpr(Context)) {
-    Diag(Arg->getBeginLoc(), diag::err_preserve_field_info_not_const)
-        << 2 << Arg->getSourceRange();
+  if (InvalidArg) {
+    Diag(Arg->getBeginLoc(), kind) << 1 << Arg->getSourceRange();
     return true;
   }
 
+  if (ReturnUnsignedInt)
     TheCall->setType(Context.UnsignedIntTy);
+  else
+    TheCall->setType(Context.UnsignedLongTy);
   return false;
 }
 
 bool Sema::CheckHexagonBuiltinArgument(unsigned BuiltinID, CallExpr *TheCall) {
   struct ArgInfo {
     uint8_t OpNum;
     bool IsSigned;
     uint8_t BitWidth;

clang/test/CodeGen/builtins-bpf-preserve-field-info-3.c

+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+#define _(x, y) (__builtin_preserve_type_info((x), (y)))
+
+struct s {
+  char a;
+};
+typedef int __int;
+enum AA {
+  VAL1 = 1,
+  VAL2 = 2,
+};
+
+unsigned unit1() {
+  struct s v = {};
+  return _(v, 0) + _(*(struct s *)0, 0);
+}
+
+// CHECK: call i32 @llvm.bpf.preserve.type.info(i32 0, i64 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_S:[0-9]+]]
+// CHECK: call i32 @llvm.bpf.preserve.type.info(i32 1, i64 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_S]]
+
+unsigned unit2() {
+  __int n;
+  return _(n, 1) + _(*(__int *)0, 1);
+}
+
+// CHECK: call i32 @llvm.bpf.preserve.type.info(i32 2, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[TYPEDEF_INT:[0-9]+]]
+// CHECK: call i32 @llvm.bpf.preserve.type.info(i32 3, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[TYPEDEF_INT]]
+
+unsigned unit3() {
+  enum AA t;
+  return _(t, 0) + _(*(enum AA *)0, 1);
+}
+
+// CHECK: call i32 @llvm.bpf.preserve.type.info(i32 4, i64 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[ENUM_AA:[0-9]+]]
+// CHECK: call i32 @llvm.bpf.preserve.type.info(i32 5, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[ENUM_AA]]
+
+// CHECK: ![[ENUM_AA]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "AA"
+// CHECK: ![[TYPEDEF_INT]] = !DIDerivedType(tag: DW_TAG_typedef, name: "__int"
+// CHECK: ![[STRUCT_S]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "s"

clang/test/CodeGen/builtins-bpf-preserve-field-info-4.c

+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+#define _(x, y) (__builtin_preserve_enum_value((x), (y)))
+
+enum AA {
+  VAL1 = 2,
+  VAL2 = 0xffffffff80000000UL,
+};
+typedef enum { VAL10 = -2, VAL11 = 0xffff8000, }  __BB;
+
+unsigned unit1() {
+  return _(*(enum AA *)VAL1, 0) + _(*(__BB *)VAL10, 1);
+}
+
+unsigned unit2() {
+  return _(*(enum AA *)VAL2, 0) + _(*(__BB *)VAL11, 1);
+}
+
+// CHECK: @0 = private unnamed_addr constant [7 x i8] c"VAL1:2\00", align 1
+// CHECK: @1 = private unnamed_addr constant [9 x i8] c"VAL10:-2\00", align 1
+// CHECK: @2 = private unnamed_addr constant [17 x i8] c"VAL2:-2147483648\00", align 1
+// CHECK: @3 = private unnamed_addr constant [17 x i8] c"VAL11:4294934528\00", align 1
+
+// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 0, i8* getelementptr inbounds ([7 x i8], [7 x i8]* @0, i32 0, i32 0), i64 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[ENUM_AA:[0-9]+]]
+// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 1, i8* getelementptr inbounds ([9 x i8], [9 x i8]* @1, i32 0, i32 0), i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[TYPEDEF_ENUM:[0-9]+]]
+
+// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 2, i8* getelementptr inbounds ([17 x i8], [17 x i8]* @2, i32 0, i32 0), i64 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[ENUM_AA]]
+// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 3, i8* getelementptr inbounds ([17 x i8], [17 x i8]* @3, i32 0, i32 0), i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[TYPEDEF_ENUM]]
+
+// CHECK: ![[ENUM_AA]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "AA"
+// CHECK: ![[TYPEDEF_ENUM]] = !DIDerivedType(tag: DW_TAG_typedef, name: "__BB"

clang/test/Sema/builtins-bpf.c

 // RUN: %clang_cc1 -x c -triple bpf-pc-linux-gnu -dwarf-version=4 -fsyntax-only -verify %s
 
-struct s { int a; int b[4]; int c:1; };
-union u { int a; int b[4]; int c:1; };
+struct s {
+  int a;
+  int b[4];
+  int c:1;
+};
+union u {
+  int a;
+  int b[4];
+  int c:1;
+};
+typedef struct {
+  int a;
+  int b;
+} __t;
+typedef int (*__f)(void);
+enum AA {
+  VAL1 = 10,
+  VAL2 = 0xffffffff80000000UL,
+};
+typedef enum {
+  VAL10 = 10,
+  VAL11 = 11,
+} __BB;
 
 unsigned invalid1(const int *arg) {
   return __builtin_preserve_field_info(arg, 1); // expected-error {{__builtin_preserve_field_info argument 1 not a field access}}
 }
 
 unsigned invalid2(const int *arg) {
   return __builtin_preserve_field_info(*arg, 1); // expected-error {{__builtin_preserve_field_info argument 1 not a field access}}
 }

 
 unsigned invalid10(struct s *arg) {
   return __builtin_preserve_field_info(arg->a, arg); // expected-error {{__builtin_preserve_field_info argument 2 not a constant}}
 }
 
 unsigned invalid11(struct s *arg, int info_kind) {
   return __builtin_preserve_field_info(arg->a, info_kind); // expected-error {{__builtin_preserve_field_info argument 2 not a constant}}
 }
+
+unsigned valid12() {
+  const struct s t;
+  return __builtin_preserve_type_info(t, 0) +
+         __builtin_preserve_type_info(*(struct s *)0, 1);
+}
+
+unsigned valid13() {
+  __t t;
+  return __builtin_preserve_type_info(t, 1) +
+         __builtin_preserve_type_info(*(__t *)0, 0);
+}
+
+unsigned valid14() {
+  enum AA t;
+  return __builtin_preserve_type_info(t, 0) +
+         __builtin_preserve_type_info(*(enum AA *)0, 1);
+}
+
+unsigned valid15() {
+  return __builtin_preserve_enum_value(*(enum AA *)VAL1, 1) +
+         __builtin_preserve_enum_value(*(enum AA *)VAL2, 1);
+}
+
+unsigned invalid16() {
+  return __builtin_preserve_enum_value(*(enum AA *)0, 1); // expected-error {{__builtin_preserve_enum_value argument 1 invalid}}
+}
+
+unsigned invalid17() {
+  return __builtin_preserve_enum_value(*(enum AA *)VAL10, 1); // expected-error {{__builtin_preserve_enum_value argument 1 invalid}}
+}
+
+unsigned invalid18(struct s *arg) {
+  return __builtin_preserve_type_info(arg->a + 2, 0); // expected-error {{__builtin_preserve_type_info argument 1 invalid}}
+}

llvm/include/llvm/IR/IntrinsicsBPF.td

let TargetPrefix = "bpf" in {  // All intrinsics start with "llvm.bpf."
   def int_bpf_pseudo : GCCBuiltin<"__builtin_bpf_pseudo">,
               Intrinsic<[llvm_i64_ty], [llvm_i64_ty, llvm_i64_ty]>;
   def int_bpf_preserve_field_info : GCCBuiltin<"__builtin_bpf_preserve_field_info">,
               Intrinsic<[llvm_i32_ty], [llvm_anyptr_ty, llvm_i64_ty],
               [IntrNoMem, ImmArg<ArgIndex<1>>]>;
   def int_bpf_btf_type_id : GCCBuiltin<"__builtin_bpf_btf_type_id">,
               Intrinsic<[llvm_i32_ty], [llvm_any_ty, llvm_any_ty, llvm_i64_ty],
               [IntrNoMem]>;
+  def int_bpf_preserve_type_info : GCCBuiltin<"__builtin_bpf_preserve_type_info">,
+              Intrinsic<[llvm_i32_ty], [llvm_i32_ty, llvm_i64_ty],
+              [IntrNoMem]>;
+  def int_bpf_preserve_enum_value : GCCBuiltin<"__builtin_bpf_preserve_enum_value">,
+              Intrinsic<[llvm_i64_ty], [llvm_i32_ty, llvm_ptr_ty, llvm_i64_ty],
+              [IntrNoMem]>;
 }