[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-07-24 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song abandoned this revision.
yonghong-song added a comment.

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


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-07-11 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

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


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-07-10 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

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


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-07-10 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert reopened this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

The test coverage here is not acceptable:

1. The command line of the tests is far from what it should be (see other 
tests).
2. The check lines do little to prevent regressions in the future.
3. There are no test for wrong usage, errors, ...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-07-08 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL365438: [BPF] Preserve debuginfo array/union/struct 
type/access index (authored by yhs, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61809?vs=207704=208591#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61809

Files:
  cfe/trunk/docs/LanguageExtensions.rst
  cfe/trunk/include/clang/Basic/Builtins.def
  cfe/trunk/lib/CodeGen/CGBuilder.h
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/CodeGen/bpf-preserve-access-index-2.c
  cfe/trunk/test/CodeGen/bpf-preserve-access-index.c

Index: cfe/trunk/docs/LanguageExtensions.rst
===
--- cfe/trunk/docs/LanguageExtensions.rst
+++ cfe/trunk/docs/LanguageExtensions.rst
@@ -1950,6 +1950,35 @@
 These builtins are intended for use in the implementation of ``std::allocator``
 and other similar allocation libraries, and are only available in C++.
 
+``__builtin_preserve_access_index``
+---
+
+``__builtin_preserve_access_index`` specifies a code section where
+array subscript access and structure/union member access are relocatable
+under bpf compile-once run-everywhere framework. Debuginfo (typically
+with ``-g``) is needed, otherwise, the compiler will exit with an error.
+
+**Syntax**:
+
+.. code-block:: c
+
+  const void * __builtin_preserve_access_index(const void * ptr)
+
+**Example of Use**:
+
+.. code-block:: c
+
+  struct t {
+int i;
+int j;
+union {
+  int a;
+  int b;
+} c[4];
+  };
+  struct t *v = ...;
+  const void *pb =__builtin_preserve_access_index(>c[3].b);
+
 Multiprecision Arithmetic Builtins
 --
 
Index: cfe/trunk/include/clang/Basic/Builtins.def
===
--- cfe/trunk/include/clang/Basic/Builtins.def
+++ cfe/trunk/include/clang/Basic/Builtins.def
@@ -1449,6 +1449,7 @@
 BUILTIN(__builtin_operator_delete, "vv*", "tn")
 BUILTIN(__builtin_char_memchr, "c*cC*iz", "n")
 BUILTIN(__builtin_dump_struct, "ivC*v*", "tn")
+BUILTIN(__builtin_preserve_access_index, "vC*vC*", "nU")
 
 // Safestack builtins
 BUILTIN(__builtin___get_unsafe_stack_start, "v*", "Fn")
Index: cfe/trunk/test/CodeGen/bpf-preserve-access-index-2.c
===
--- cfe/trunk/test/CodeGen/bpf-preserve-access-index-2.c
+++ cfe/trunk/test/CodeGen/bpf-preserve-access-index-2.c
@@ -0,0 +1,22 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (x)
+
+const void *test(struct t *arg) {
+  return _(>c[3].b);
+}
+
+// CHECK-NOT: llvm.preserve.struct.access.index
+// CHECK-NOT: llvm.preserve.array.access.index
+// CHECK-NOT: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
Index: cfe/trunk/test/CodeGen/bpf-preserve-access-index.c
===
--- cfe/trunk/test/CodeGen/bpf-preserve-access-index.c
+++ cfe/trunk/test/CodeGen/bpf-preserve-access-index.c
@@ -0,0 +1,22 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (__builtin_preserve_access_index(x))
+
+const void *test(struct t *arg) {
+  return _(>c[3].b);
+}
+
+// CHECK: llvm.preserve.struct.access.index
+// CHECK: llvm.preserve.array.access.index
+// CHECK: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
Index: cfe/trunk/lib/CodeGen/CGBuilder.h
===
--- cfe/trunk/lib/CodeGen/CGBuilder.h
+++ cfe/trunk/lib/CodeGen/CGBuilder.h
@@ -298,6 +298,21 @@
 return CreateMemSet(Dest.getPointer(), Value, Size,
 Dest.getAlignment().getQuantity(), IsVolatile);
   }
+
+  using CGBuilderBaseTy::CreatePreserveStructAccessIndex;
+  Address CreatePreserveStructAccessIndex(Address Addr,
+  unsigned Index,
+  unsigned FieldIndex,
+  llvm::MDNode *DbgInfo) {
+llvm::StructType *ElTy = cast(Addr.getElementType());
+const llvm::DataLayout  = BB->getParent()->getParent()->getDataLayout();
+const llvm::StructLayout *Layout = DL.getStructLayout(ElTy);
+   

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-07-05 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

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


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-07-02 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

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


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-07-02 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 207704.
yonghong-song added a comment.

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


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809

Files:
  docs/LanguageExtensions.rst
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuilder.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaChecking.cpp
  test/CodeGen/bpf-preserve-access-index-2.c
  test/CodeGen/bpf-preserve-access-index.c

Index: test/CodeGen/bpf-preserve-access-index.c
===
--- /dev/null
+++ test/CodeGen/bpf-preserve-access-index.c
@@ -0,0 +1,27 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+
+struct t {
+  int i:1;
+  int j:2;
+  int :3;
+  union {
+   int a;
+   int :32;
+   int b;
+  } c[4];
+};
+
+#define _(x) (__builtin_preserve_access_index(x))
+
+const void *test(struct t *arg) {
+  return _(>c[3].b);
+}
+
+// CHECK: llvm.preserve.struct.access.index
+// CHECK: (%struct.t* %0, i32 1, i32 2)
+// CHECK: llvm.preserve.array.access.index
+// CHECK: ([4 x %union.anon]* %2, i32 1, i32 3)
+// CHECK: llvm.preserve.union.access.index
+// CHECK: (%union.anon* %3, i32 1)
+// CHECK-NOT: __builtin_preserve_access_index
Index: test/CodeGen/bpf-preserve-access-index-2.c
===
--- /dev/null
+++ test/CodeGen/bpf-preserve-access-index-2.c
@@ -0,0 +1,24 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+
+struct t {
+  int i:1;
+  int j:2;
+  int :3;
+  union {
+   int a;
+   int :32;
+   int b;
+  } c[4];
+};
+
+#define _(x) (x)
+
+const void *test(struct t *arg) {
+  return _(>c[3].b);
+}
+
+// CHECK-NOT: llvm.preserve.struct.access.index
+// CHECK-NOT: llvm.preserve.array.access.index
+// CHECK-NOT: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -191,6 +191,16 @@
   return false;
 }
 
+/// Check the number of arguments, and set the result type to
+/// the argument type.
+static bool SemaBuiltinPreserveAI(Sema , CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+return true;
+
+  TheCall->setType(TheCall->getArg(0)->getType());
+  return false;
+}
+
 static bool SemaBuiltinOverflow(Sema , CallExpr *TheCall) {
   if (checkArgCount(S, TheCall, 3))
 return true;
@@ -1409,6 +1419,10 @@
 TheCall->setType(Context.IntTy);
 break;
   }
+  case Builtin::BI__builtin_preserve_access_index:
+if (SemaBuiltinPreserveAI(*this, TheCall))
+  return ExprError();
+break;
   case Builtin::BI__builtin_call_with_static_chain:
 if (SemaBuiltinCallWithStaticChain(*this, TheCall))
   return ExprError();
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -480,6 +480,10 @@
   /// finally block or filter expression.
   bool IsOutlinedSEHHelper = false;
 
+  /// True if CodeGen currently emits code inside presereved access index
+  /// region.
+  bool IsInPreservedAIRegion = false;
+
   const CodeGen::CGBlockInfo *BlockInfo = nullptr;
   llvm::Value *BlockPointer = nullptr;
 
@@ -2648,6 +2652,9 @@
   /// Converts Location to a DebugLoc, if debug information is enabled.
   llvm::DebugLoc SourceLocToDebugLoc(SourceLocation Location);
 
+  /// Get the record field index as represented in debug info.
+  unsigned getDebugInfoFIndex(const RecordDecl *Rec, unsigned FieldIndex);
+
 
   //======//
   //Declaration Emission
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
@@ -3418,8 +3419,20 @@
   CharUnits eltAlign =
 getArrayElementAlign(addr.getAlignment(), indices.back(), eltSize);
 
-  llvm::Value *eltPtr = emitArraySubscriptGEP(
-  CGF, addr.getPointer(), indices, inbounds, signedIndices, loc, name);
+  llvm::Value *eltPtr;
+  auto LastIndex = dyn_cast(indices.back());
+  if (!CGF.IsInPreservedAIRegion || 

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-07-02 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 207644.
yonghong-song edited the summary of this revision.
yonghong-song added a comment.

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


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809

Files:
  docs/LanguageExtensions.rst
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuilder.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaChecking.cpp
  test/CodeGen/bpf-preserve-access-index-2.c
  test/CodeGen/bpf-preserve-access-index.c

Index: test/CodeGen/bpf-preserve-access-index.c
===
--- /dev/null
+++ test/CodeGen/bpf-preserve-access-index.c
@@ -0,0 +1,22 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (__builtin_preserve_access_index(x))
+
+const void *test(struct t *arg) {
+  return _(>c[3].b);
+}
+
+// CHECK: llvm.preserve.struct.access.index
+// CHECK: llvm.preserve.array.access.index
+// CHECK: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
Index: test/CodeGen/bpf-preserve-access-index-2.c
===
--- /dev/null
+++ test/CodeGen/bpf-preserve-access-index-2.c
@@ -0,0 +1,22 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (x)
+
+const void *test(struct t *arg) {
+  return _(>c[3].b);
+}
+
+// CHECK-NOT: llvm.preserve.struct.access.index
+// CHECK-NOT: llvm.preserve.array.access.index
+// CHECK-NOT: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -191,6 +191,16 @@
   return false;
 }
 
+/// Check the number of arguments, and set the result type to
+/// the argument type.
+static bool SemaBuiltinPreserveAI(Sema , CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+return true;
+
+  TheCall->setType(TheCall->getArg(0)->getType());
+  return false;
+}
+
 static bool SemaBuiltinOverflow(Sema , CallExpr *TheCall) {
   if (checkArgCount(S, TheCall, 3))
 return true;
@@ -1409,6 +1419,10 @@
 TheCall->setType(Context.IntTy);
 break;
   }
+  case Builtin::BI__builtin_preserve_access_index:
+if (SemaBuiltinPreserveAI(*this, TheCall))
+  return ExprError();
+break;
   case Builtin::BI__builtin_call_with_static_chain:
 if (SemaBuiltinCallWithStaticChain(*this, TheCall))
   return ExprError();
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -480,6 +480,10 @@
   /// finally block or filter expression.
   bool IsOutlinedSEHHelper = false;
 
+  /// True if CodeGen currently emits code inside presereved access index
+  /// region.
+  bool IsInPreservedAIRegion = false;
+
   const CodeGen::CGBlockInfo *BlockInfo = nullptr;
   llvm::Value *BlockPointer = nullptr;
 
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
@@ -3418,8 +3419,20 @@
   CharUnits eltAlign =
 getArrayElementAlign(addr.getAlignment(), indices.back(), eltSize);
 
-  llvm::Value *eltPtr = emitArraySubscriptGEP(
-  CGF, addr.getPointer(), indices, inbounds, signedIndices, loc, name);
+  llvm::Value *eltPtr;
+  auto LastIndex = dyn_cast(indices.back());
+  if (!CGF.IsInPreservedAIRegion || !LastIndex) {
+eltPtr = emitArraySubscriptGEP(
+CGF, addr.getPointer(), indices, inbounds, signedIndices,
+loc, name);
+  } else {
+// Remember the original array subscript for bpf target
+unsigned idx = LastIndex->getZExtValue();
+eltPtr = CGF.Builder.CreatePreserveArrayAccessIndex(addr.getPointer(),
+indices.size() - 1,
+idx);
+  }
+
   return Address(eltPtr, eltAlign);
 }
 
@@ -3908,6 +3921,19 @@
   return CGF.Builder.CreateStructGEP(base, idx, 

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-07-01 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 207475.
yonghong-song added a comment.

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


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809

Files:
  docs/LanguageExtensions.rst
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuilder.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaChecking.cpp
  test/CodeGen/bpf-preserve-access-index-2.c
  test/CodeGen/bpf-preserve-access-index.c

Index: test/CodeGen/bpf-preserve-access-index.c
===
--- /dev/null
+++ test/CodeGen/bpf-preserve-access-index.c
@@ -0,0 +1,22 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (__builtin_preserve_access_index(x))
+
+void *test(struct t *arg) {
+  return _(>c[3].b);
+}
+
+// CHECK: llvm.preserve.struct.access.index
+// CHECK: llvm.preserve.array.access.index
+// CHECK: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
Index: test/CodeGen/bpf-preserve-access-index-2.c
===
--- /dev/null
+++ test/CodeGen/bpf-preserve-access-index-2.c
@@ -0,0 +1,22 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (x)
+
+void *test(struct t *arg) {
+  return _(>c[3].b);
+}
+
+// CHECK-NOT: llvm.preserve.struct.access.index
+// CHECK-NOT: llvm.preserve.array.access.index
+// CHECK-NOT: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -191,6 +191,16 @@
   return false;
 }
 
+/// Check the number of arguments, and set the result type to
+/// the argument type.
+static bool SemaBuiltinPreserveAI(Sema , CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+return true;
+
+  TheCall->setType(TheCall->getArg(0)->getType());
+  return false;
+}
+
 static bool SemaBuiltinOverflow(Sema , CallExpr *TheCall) {
   if (checkArgCount(S, TheCall, 3))
 return true;
@@ -1409,6 +1419,10 @@
 TheCall->setType(Context.IntTy);
 break;
   }
+  case Builtin::BI__builtin_preserve_access_index:
+if (SemaBuiltinPreserveAI(*this, TheCall))
+  return ExprError();
+break;
   case Builtin::BI__builtin_call_with_static_chain:
 if (SemaBuiltinCallWithStaticChain(*this, TheCall))
   return ExprError();
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -480,6 +480,10 @@
   /// finally block or filter expression.
   bool IsOutlinedSEHHelper = false;
 
+  /// True if CodeGen currently emits code inside presereved access index
+  /// region.
+  bool IsInPreservedAIRegion = false;
+
   const CodeGen::CGBlockInfo *BlockInfo = nullptr;
   llvm::Value *BlockPointer = nullptr;
 
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
@@ -3418,8 +3419,20 @@
   CharUnits eltAlign =
 getArrayElementAlign(addr.getAlignment(), indices.back(), eltSize);
 
-  llvm::Value *eltPtr = emitArraySubscriptGEP(
-  CGF, addr.getPointer(), indices, inbounds, signedIndices, loc, name);
+  llvm::Value *eltPtr;
+  auto LastIndex = dyn_cast(indices.back());
+  if (!CGF.IsInPreservedAIRegion || !LastIndex) {
+eltPtr = emitArraySubscriptGEP(
+CGF, addr.getPointer(), indices, inbounds, signedIndices,
+loc, name);
+  } else {
+// Remember the original array subscript for bpf target
+unsigned idx = LastIndex->getZExtValue();
+eltPtr = CGF.Builder.CreatePreserveArrayAccessIndex(addr.getPointer(),
+indices.size() - 1,
+idx);
+  }
+
   return Address(eltPtr, eltAlign);
 }
 
@@ -3908,6 +3921,19 @@
   return CGF.Builder.CreateStructGEP(base, idx, field->getName());
 }
 

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-06-27 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song marked 2 inline comments as done.
yonghong-song added a comment.

@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!




Comment at: docs/LanguageExtensions.rst:1958
+array subscript access and structure/union member access are preserved with
+IR intrinsics ``preserve_array_access_index``, ``preserve_union_access_index``
+and ``preserve_struct_access_index``, instead of IR GetElementPtr instructions.

efriedma wrote:
> "preserved with the IR intrinsics" isn't really useful; this is the user's 
> manual, not a developer guide to LLVM internals. Probably better to say what 
> it enables from the user's perspective: the CO-RE feature for BPF targets.
Will reword to be more towards users in the next revision.



Comment at: docs/LanguageExtensions.rst:1960
+and ``preserve_struct_access_index``, instead of IR GetElementPtr instructions.
+``__builtin_preserve_access_index`` takes effect only when debuginfo (typically
+with ``-g``) is available since debuginfo is used as IR intrinsic metadata

efriedma wrote:
> I would rather not have __builtin_preserve_access_index fail to do anything 
> when debug info is disabled. If it's hard to fix, making it a hard error is 
> probably okay.
The IR intrinsics needs to have debuginfo as the metadata so that the 
user-level access info can be reconstructed. If no debug info, just IR 
intrinsics without debuginfo is less useful. So let me make a hard error.

We can relax it later if IR intrinsics without deebuginfo metadata becomes 
workable/useful.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-06-27 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

The new approach to tracking expressions inside of 
__builtin_preserve_access_index seems okay.

Please let @rsmith comment since he looked at this before.




Comment at: docs/LanguageExtensions.rst:1958
+array subscript access and structure/union member access are preserved with
+IR intrinsics ``preserve_array_access_index``, ``preserve_union_access_index``
+and ``preserve_struct_access_index``, instead of IR GetElementPtr instructions.

"preserved with the IR intrinsics" isn't really useful; this is the user's 
manual, not a developer guide to LLVM internals. Probably better to say what it 
enables from the user's perspective: the CO-RE feature for BPF targets.



Comment at: docs/LanguageExtensions.rst:1960
+and ``preserve_struct_access_index``, instead of IR GetElementPtr instructions.
+``__builtin_preserve_access_index`` takes effect only when debuginfo (typically
+with ``-g``) is available since debuginfo is used as IR intrinsic metadata

I would rather not have __builtin_preserve_access_index fail to do anything 
when debug info is disabled. If it's hard to fix, making it a hard error is 
probably okay.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-06-27 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

@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!


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-06-24 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song marked an inline comment as not done.
yonghong-song added a comment.

@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!


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-06-24 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 206274.
yonghong-song added a comment.

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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809

Files:
  docs/LanguageExtensions.rst
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuilder.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaChecking.cpp
  test/CodeGen/bpf-preserve-access-index-2.c
  test/CodeGen/bpf-preserve-access-index.c

Index: test/CodeGen/bpf-preserve-access-index.c
===
--- /dev/null
+++ test/CodeGen/bpf-preserve-access-index.c
@@ -0,0 +1,28 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -O2 -o - | FileCheck --check-prefix=CHECK-NODBG %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -O2 -o - | FileCheck --check-prefix=CHECK-NODBG %s
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (__builtin_preserve_access_index(x))
+
+void *test(struct t *arg) {
+  return _(>c[3].b);
+}
+
+// CHECK: llvm.preserve.struct.access.index
+// CHECK: llvm.preserve.array.access.index
+// CHECK: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
+// CHECK-NODBG-NOT: llvm.preserve.struct.access.index
+// CHECK-NODBG-NOT: llvm.preserve.array.access.index
+// CHECK-NODBG-NOT: llvm.preserve.union.access.index
+// CHECK-NODBG-NOT: __builtin_preserve_access_index
Index: test/CodeGen/bpf-preserve-access-index-2.c
===
--- /dev/null
+++ test/CodeGen/bpf-preserve-access-index-2.c
@@ -0,0 +1,22 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (x)
+
+void *test(struct t *arg) {
+  return _(>c[3].b);
+}
+
+// CHECK-NOT: llvm.preserve.struct.access.index
+// CHECK-NOT: llvm.preserve.array.access.index
+// CHECK-NOT: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -191,6 +191,16 @@
   return false;
 }
 
+/// Check the number of arguments, and set the result type to
+/// the argument type.
+static bool SemaBuiltinPreserveAI(Sema , CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+return true;
+
+  TheCall->setType(TheCall->getArg(0)->getType());
+  return false;
+}
+
 static bool SemaBuiltinOverflow(Sema , CallExpr *TheCall) {
   if (checkArgCount(S, TheCall, 3))
 return true;
@@ -1409,6 +1419,10 @@
 TheCall->setType(Context.IntTy);
 break;
   }
+  case Builtin::BI__builtin_preserve_access_index:
+if (SemaBuiltinPreserveAI(*this, TheCall))
+  return ExprError();
+break;
   case Builtin::BI__builtin_call_with_static_chain:
 if (SemaBuiltinCallWithStaticChain(*this, TheCall))
   return ExprError();
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -480,6 +480,10 @@
   /// finally block or filter expression.
   bool IsOutlinedSEHHelper = false;
 
+  /// True if CodeGen currently emits code inside presereved access index
+  /// region.
+  bool IsInPreservedAIRegion = false;
+
   const CodeGen::CGBlockInfo *BlockInfo = nullptr;
   llvm::Value *BlockPointer = nullptr;
 
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
@@ -3418,8 +3419,20 @@
   CharUnits eltAlign =
 getArrayElementAlign(addr.getAlignment(), indices.back(), eltSize);
 
-  llvm::Value *eltPtr = emitArraySubscriptGEP(
-  CGF, addr.getPointer(), indices, inbounds, signedIndices, loc, name);
+  llvm::Value *eltPtr;
+  auto LastIndex = dyn_cast(indices.back());
+  if (!CGF.IsInPreservedAIRegion || !LastIndex) {
+eltPtr = emitArraySubscriptGEP(
+CGF, addr.getPointer(), indices, inbounds, signedIndices,
+loc, name);
+  } else {
+// Remember the original array 

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-06-19 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song marked an inline comment as done.
yonghong-song added a comment.

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

> can we really expect the user to know which expressions to apply this to?

Yes, this is specifically targeting some bpf helper calls like bpf_probe_read. 
So users know which expressions to apply.

> I'd like to see an actual specification for this in 
> docs/LanguageExtensions.rst at some point.

I will find a place to put this into docs/LanguageExtensions.rst.




Comment at: lib/CodeGen/CGExpr.cpp:663
+  while (true) {
+const auto  = getContext().getParents(*E);
+if (Parents.size() != 1)

efriedma wrote:
> I'm not sure you can use getParents like this safely... it's not really meant 
> for use inside of clang semantic analysis/code generation, and I don't think 
> we recompute it as the AST changes.
Good point. Let me check whether I can traverse AST instead.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-06-11 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

The general idea of restricting the intrinsics to specific contexts makes 
sense.  I'm not sure it makes sense to mark expressions, as opposed to types, 
though; can we really expect the user to know which expressions to apply this 
to?

I'd like to see an actual specification for this in docs/LanguageExtensions.rst 
at some point.

Other than that, generally seems okay.




Comment at: lib/CodeGen/CGExpr.cpp:663
+  while (true) {
+const auto  = getContext().getParents(*E);
+if (Parents.size() != 1)

I'm not sure you can use getParents like this safely... it's not really meant 
for use inside of clang semantic analysis/code generation, and I don't think we 
recompute it as the AST changes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-06-11 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

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


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-05-28 Thread Alexei Starovoitov via Phabricator via cfe-commits
ast accepted this revision.
ast added a comment.

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-05-28 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 201793.
yonghong-song added a comment.

remove bpf offsetreloc option and use FileCheck in the test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuilder.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaChecking.cpp
  test/CodeGen/bpf-offsetreloc.c

Index: test/CodeGen/bpf-offsetreloc.c
===
--- /dev/null
+++ test/CodeGen/bpf-offsetreloc.c
@@ -0,0 +1,22 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (__builtin_preserve_access_index(x))
+
+void *test(struct t *arg) {
+  return _(>c[3].b);
+}
+
+// CHECK: llvm.preserve.struct.access.index
+// CHECK: llvm.preserve.array.access.index
+// CHECK: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -190,6 +190,16 @@
   return false;
 }
 
+/// Check the number of arguments, and set the result type to
+/// the argument type.
+static bool SemaBuiltinPreserveAI(Sema , CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+return true;
+
+  TheCall->setType(TheCall->getArg(0)->getType());
+  return false;
+}
+
 static bool SemaBuiltinOverflow(Sema , CallExpr *TheCall) {
   if (checkArgCount(S, TheCall, 3))
 return true;
@@ -1407,6 +1417,10 @@
 TheCall->setType(Context.IntTy);
 break;
   }
+  case Builtin::BI__builtin_preserve_access_index:
+if (SemaBuiltinPreserveAI(*this, TheCall))
+  return ExprError();
+break;
   case Builtin::BI__builtin_call_with_static_chain:
 if (SemaBuiltinCallWithStaticChain(*this, TheCall))
   return ExprError();
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1028,6 +1028,10 @@
 store->setAlignment(addr.getAlignment().getQuantity());
   }
 
+  /// isPreserveDIAccessIndexNeeded - Return true if it is needed to
+  /// preserve the Debuginfo access index.
+  bool isPreserveDIAccessIndexNeeded(const Expr *E);
+
   /// An RAII object to record that we're evaluating a statement
   /// expression.
   class StmtExprEvaluation {
@@ -3559,7 +3563,8 @@
 
   llvm::Value *EmitIvarOffset(const ObjCInterfaceDecl *Interface,
   const ObjCIvarDecl *Ivar);
-  LValue EmitLValueForField(LValue Base, const FieldDecl* Field);
+  LValue EmitLValueForField(LValue Base, const FieldDecl* Field,
+const Expr *E = nullptr);
   LValue EmitLValueForLambdaField(const FieldDecl *Field);
 
   /// EmitLValueForFieldInitialization - Like EmitLValueForField, except that
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
@@ -649,6 +650,52 @@
  SanOpts.has(SanitizerKind::Vptr);
 }
 
+/// The expression E is a candidate for preserving debuginfo access index
+/// if it is inside an __builtin_preserve_access_index intrinsic call.
+bool CodeGenFunction::isPreserveDIAccessIndexNeeded(const Expr *E) {
+  if (!E)
+return false;
+
+  if (!getDebugInfo())
+return false;
+
+  while (true) {
+const auto  = getContext().getParents(*E);
+if (Parents.size() != 1)
+  return false;
+
+E = Parents[0].get();
+if (!E)
+  return false;
+
+// Check whether E is a BI__builtin_preserve_access_index
+// intrinsic call.
+const auto *CE = dyn_cast(E);
+if (CE) {
+  // Callee must a builtin function.
+  const Expr *Callee = CE->getCallee()->IgnoreParens();
+  auto ICE = dyn_cast(Callee);
+  if (!ICE)
+return false;
+  if (ICE->getCastKind() != CK_BuiltinFnToFnPtr)
+return false;
+
+  auto DRE = dyn_cast(ICE->getSubExpr());
+  if (!DRE)
+return false;
+
+  if (auto FD = dyn_cast(DRE->getDecl())) {
+if (FD->getBuiltinID() == Builtin::BI__builtin_preserve_access_index)
+  return true;
+  }
+
+  break;
+}
+  }
+
+  return false;
+}
+
 void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
 llvm::Value *Ptr, QualType Ty,
 

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-05-28 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

@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?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-05-23 Thread Alexei Starovoitov via Phabricator via cfe-commits
ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

from kernel, libbpf, tooling and user experience points of view this approach 
looks the best to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-05-23 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song marked an inline comment as done.
yonghong-song added inline comments.



Comment at: test/CodeGen/bpf-offsetreloc.c:2-4
+// RUN: grep "llvm.preserve.struct.access.index" %t1
+// RUN: grep "llvm.preserve.array.access.index" %t1
+// RUN: grep "llvm.preserve.union.access.index" %t1

lebedev.ri wrote:
> This looks like a bad test.
> Can't you use FileCheck?
> I'd think there is more to test than a single tiny test..
Thanks. I will add more tests with FileCheck once we get an agreement for the 
general approach.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-05-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri resigned from this revision.
lebedev.ri added a comment.

Don't think i will be of any help here.




Comment at: test/CodeGen/bpf-offsetreloc.c:2-4
+// RUN: grep "llvm.preserve.struct.access.index" %t1
+// RUN: grep "llvm.preserve.array.access.index" %t1
+// RUN: grep "llvm.preserve.union.access.index" %t1

This looks like a bad test.
Can't you use FileCheck?
I'd think there is more to test than a single tiny test..


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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


[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

2019-05-21 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 200639.
yonghong-song retitled this revision from "[BPF] Preserve debuginfo 
array/union/struct type name/access index" to "[BPF] Preserve debuginfo 
array/union/struct type/access index".
yonghong-song edited the summary of this revision.
yonghong-song added a comment.

add clang builtin __builtin_preserve_access_index


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809

Files:
  include/clang/Basic/Builtins.def
  lib/Basic/Targets/BPF.h
  lib/CodeGen/CGBuilder.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaChecking.cpp
  test/CodeGen/bpf-offsetreloc.c

Index: test/CodeGen/bpf-offsetreloc.c
===
--- /dev/null
+++ test/CodeGen/bpf-offsetreloc.c
@@ -0,0 +1,19 @@
+// RUN: %clang -target bpf -emit-llvm -S -g -O2 -o - %s > %t1
+// RUN: grep "llvm.preserve.struct.access.index" %t1
+// RUN: grep "llvm.preserve.array.access.index" %t1
+// RUN: grep "llvm.preserve.union.access.index" %t1
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (__builtin_preserve_access_index(x))
+
+void *test(struct t *arg) {
+  return _(>c[3].b);
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -190,6 +190,16 @@
   return false;
 }
 
+/// Check the number of arguments, and set the result type to
+/// the argument type.
+static bool SemaBuiltinPreserveAI(Sema , CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+return true;
+
+  TheCall->setType(TheCall->getArg(0)->getType());
+  return false;
+}
+
 static bool SemaBuiltinOverflow(Sema , CallExpr *TheCall) {
   if (checkArgCount(S, TheCall, 3))
 return true;
@@ -1407,6 +1417,10 @@
 TheCall->setType(Context.IntTy);
 break;
   }
+  case Builtin::BI__builtin_preserve_access_index:
+if (SemaBuiltinPreserveAI(*this, TheCall))
+  return ExprError();
+break;
   case Builtin::BI__builtin_call_with_static_chain:
 if (SemaBuiltinCallWithStaticChain(*this, TheCall))
   return ExprError();
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1027,6 +1027,10 @@
 store->setAlignment(addr.getAlignment().getQuantity());
   }
 
+  /// isPreserveDIAccessIndexNeeded - Return true if it is needed to
+  /// preserve the Debuginfo access index.
+  bool isPreserveDIAccessIndexNeeded(const Expr *E);
+
   /// An RAII object to record that we're evaluating a statement
   /// expression.
   class StmtExprEvaluation {
@@ -3545,7 +3549,8 @@
 
   llvm::Value *EmitIvarOffset(const ObjCInterfaceDecl *Interface,
   const ObjCIvarDecl *Ivar);
-  LValue EmitLValueForField(LValue Base, const FieldDecl* Field);
+  LValue EmitLValueForField(LValue Base, const FieldDecl* Field,
+const Expr *E = nullptr);
   LValue EmitLValueForLambdaField(const FieldDecl *Field);
 
   /// EmitLValueForFieldInitialization - Like EmitLValueForField, except that
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
@@ -649,6 +650,52 @@
  SanOpts.has(SanitizerKind::Vptr);
 }
 
+/// The expression E is a candidate for preserving debuginfo access index
+/// if it is inside an __builtin_preserve_access_index intrinsic call.
+bool CodeGenFunction::isPreserveDIAccessIndexNeeded(const Expr *E) {
+  if (!E)
+return false;
+
+  if (!getDebugInfo())
+return false;
+
+  while (true) {
+const auto  = getContext().getParents(*E);
+if (Parents.size() != 1)
+  return false;
+
+E = Parents[0].get();
+if (!E)
+  return false;
+
+// Check whether E is a BI__builtin_preserve_access_index
+// intrinsic call.
+const auto *CE = dyn_cast(E);
+if (CE) {
+  // Callee must a builtin function.
+  const Expr *Callee = CE->getCallee()->IgnoreParens();
+  auto ICE = dyn_cast(Callee);
+  if (!ICE)
+return false;
+  if (ICE->getCastKind() != CK_BuiltinFnToFnPtr)
+return false;
+
+  auto DRE = dyn_cast(ICE->getSubExpr());
+  if (!DRE)
+return false;
+
+  if (auto FD = dyn_cast(DRE->getDecl())) {
+if (FD->getBuiltinID() == Builtin::BI__builtin_preserve_access_index)
+  return true;
+  }
+
+  break;
+}
+  }
+
+  return false;
+}
+
 void