[PATCH] D81479: [BPF] introduce __builtin_bpf_load_u32_to_ptr() intrinsic

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song abandoned this revision.
yonghong-song added a comment.

inline asm can do the work, so abandon this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81479/new/

https://reviews.llvm.org/D81479

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81479: [BPF] introduce __builtin_bpf_load_u32_to_ptr() intrinsic

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

I guess I will go with inline asm in kernel for now as llvm seems already doing 
a pretty good job parsing/understanding inline asm to integrated into 
optimization passes. A few passes like SimplifyCFG, GVN, etc. may have some 
impact but probably does not really matter for the use case here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81479/new/

https://reviews.llvm.org/D81479



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81479: [BPF] introduce __builtin_bpf_load_u32_to_ptr() intrinsic

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

In D81479#2083933 , @ast wrote:

> It feels that the same thing can be represented as inline asm.
>  What advantage builtin has?


Yes, this can be represented as an inline asm. I have a tendency not liking 
inline assembly codes in bpf programs.
But maybe for this one, we could hide inline asm in the header and provide the 
same signature like
__builtin_bpf_load_u32_to_ptr() to users.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81479/new/

https://reviews.llvm.org/D81479



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81479: [BPF] introduce __builtin_bpf_load_u32_to_ptr() intrinsic

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

It feels that the same thing can be represented as inline asm.
What advantage builtin has?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81479/new/

https://reviews.llvm.org/D81479



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81479: [BPF] introduce __builtin_bpf_load_u32_to_ptr() intrinsic

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song updated this revision to Diff 269721.
yonghong-song retitled this revision from "[BPF] introduce 
__builtin_load_u32_to_ptr() intrinsic" to "[BPF] introduce 
__builtin_bpf_load_u32_to_ptr() intrinsic".
yonghong-song edited the summary of this revision.
yonghong-song added a comment.

change builtin name from __builtin_load_u32_to_ptr to 
__builtin_bpf_load_u32_to_ptr to reflect it is bpf specific.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81479/new/

https://reviews.llvm.org/D81479

Files:
  clang/include/clang/Basic/BuiltinsBPF.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-bpf-load-u32-to-ptr.c
  clang/test/Sema/builtin-bpf-load-u32-to-ptr.c
  llvm/include/llvm/IR/IntrinsicsBPF.td

Index: llvm/include/llvm/IR/IntrinsicsBPF.td
===
--- llvm/include/llvm/IR/IntrinsicsBPF.td
+++ llvm/include/llvm/IR/IntrinsicsBPF.td
@@ -26,4 +26,6 @@
   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_load_u32_to_ptr : GCCBuiltin<"__builtin_bpf_load_u32_to_ptr">,
+  Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty, llvm_i64_ty], [IntrReadMem]>;
 }
Index: clang/test/Sema/builtin-bpf-load-u32-to-ptr.c
===
--- /dev/null
+++ clang/test/Sema/builtin-bpf-load-u32-to-ptr.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c -triple bpf-pc-linux-gnu -dwarf-version=4 -fsyntax-only -verify %s
+
+struct t { int a; int b; };
+
+void *invalid1(struct t *arg) { return __builtin_bpf_load_u32_to_ptr(arg, arg->a); } // expected-error {{__builtin_bpf_load_u32_to_ptr argument 2 not a constant}}
+void *invalid2(struct t *arg) { return __builtin_bpf_load_u32_to_ptr(arg + 4); } // expected-error {{too few arguments to function call, expected 2, have 1}}
+void *invalid3(struct t *arg) { return __builtin_bpf_load_u32_to_ptr(arg, 4, 0); } // expected-error {{too many arguments to function call, expected 2, have 3}}
+unsigned invalid4(struct t *arg) { return __builtin_bpf_load_u32_to_ptr(arg, 4); } // expected-warning {{incompatible pointer to integer conversion returning 'void *' from a function with result type 'unsigned int'}}
+
+void *valid1(struct t *arg) { return __builtin_bpf_load_u32_to_ptr(arg, 4); }
Index: clang/test/CodeGen/builtin-bpf-load-u32-to-ptr.c
===
--- /dev/null
+++ clang/test/CodeGen/builtin-bpf-load-u32-to-ptr.c
@@ -0,0 +1,8 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S %s -o - | FileCheck %s
+
+struct t { int a; int b; };
+void *test(struct t *arg) { return __builtin_bpf_load_u32_to_ptr(arg, 4); }
+
+// CHECK: define dso_local i8* @test
+// CHECK: call i8* @llvm.bpf.load.u32.to.ptr(i8* %{{[0-9a-z.]+}}, i64 4)
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2501,13 +2501,28 @@
 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_bpf_load_u32_to_ptr) &&
+ "unexpected BPF builtin");
+
+  // Generic checking has done basic checking against the
+  // signature, here only to ensure the second argument
+  // be a constant.
+  Expr *Arg;
+  if (BuiltinID == BPF::BI__builtin_bpf_load_u32_to_ptr) {
+llvm::APSInt Value;
+Arg = TheCall->getArg(1);
+if (!Arg->isIntegerConstantExpr(Value, Context)) {
+  Diag(Arg->getBeginLoc(), diag::err_bpf_load_u32_to_ptr_not_const)
+  << 2 << Arg->getSourceRange();
+  return true;
+}
+return false;
+  }
 
   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
 llvm::APSInt Value;
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -10685,7 +10685,8 @@
 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_bpf_load_u32_to_ptr) &&