[PATCH] D98134: [WIP][RFC] Introduce callback argument encoding mode into callback metadata

2021-03-08 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 updated this revision to Diff 329191.
tianshilei1992 added a comment.

update llvm doc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98134

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-callback.c
  clang/test/CodeGen/callback_annotated.c
  clang/test/CodeGen/callback_openmp.c
  clang/test/CodeGenCXX/attr-callback.cpp
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/Sema/attr-callback-broken.c
  clang/test/Sema/attr-callback.c
  clang/test/SemaCXX/attr-callback.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/AbstractCallSite.h
  llvm/include/llvm/IR/MDBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/IR/AbstractCallSite.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/test/Analysis/CallGraph/callback-calls.ll
  llvm/test/Analysis/CallGraph/ignore-callback-uses.ll
  llvm/test/Transforms/Attributor/IPConstantProp/multiple_callbacks.ll
  llvm/test/Transforms/Attributor/IPConstantProp/openmp_parallel_for.ll
  llvm/test/Transforms/Attributor/IPConstantProp/pthreads.ll
  llvm/test/Transforms/Attributor/IPConstantProp/thread_local_acs.ll
  llvm/test/Transforms/Attributor/callbacks.ll
  llvm/test/Transforms/Attributor/noundef.ll
  llvm/test/Transforms/OpenMP/parallel_deletion.ll
  llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
  llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
  llvm/test/Transforms/OpenMP/parallel_region_merging.ll
  llvm/unittests/IR/AbstractCallSiteTest.cpp
  llvm/unittests/IR/LegacyPassManagerTest.cpp

Index: llvm/unittests/IR/LegacyPassManagerTest.cpp
===
--- llvm/unittests/IR/LegacyPassManagerTest.cpp
+++ llvm/unittests/IR/LegacyPassManagerTest.cpp
@@ -768,7 +768,7 @@
"}\n"
"\n"
"!0 = !{!1}\n"
-   "!1 = !{i64 0, i64 1, i1 false}";
+   "!1 = !{i64 0, i64 0, i64 1, i1 false}";
 
   SMDiagnostic Err;
   std::unique_ptr M = parseAssemblyString(IR, Err, Context);
Index: llvm/unittests/IR/AbstractCallSiteTest.cpp
===
--- llvm/unittests/IR/AbstractCallSiteTest.cpp
+++ llvm/unittests/IR/AbstractCallSiteTest.cpp
@@ -36,7 +36,7 @@
   "}\n"
   "declare !callback !0 void @broker(i32, void (i8*, ...)*, ...)\n"
   "!0 = !{!1}\n"
-  "!1 = !{i64 1, i64 -1, i1 true}";
+  "!1 = !{i64 0, i64 1, i64 -1, i1 true}";
 
   std::unique_ptr M = parseIR(C, IR);
   ASSERT_TRUE(M);
Index: llvm/test/Transforms/OpenMP/parallel_region_merging.ll
===
--- llvm/test/Transforms/OpenMP/parallel_region_merging.ll
+++ llvm/test/Transforms/OpenMP/parallel_region_merging.ll
@@ -786,7 +786,7 @@
 
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{!2}
-!2 = !{i64 2, i64 -1, i64 -1, i1 true}
+!2 = !{i64 0, i64 2, i64 -1, i64 -1, i1 true}
 ; CHECK-LABEL: define {{[^@]+}}@merge
 ; CHECK-SAME: (i32 [[A:%.*]]) local_unnamed_addr {
 ; CHECK-NEXT:  entry:
Index: llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
===
--- llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
+++ llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
@@ -96,7 +96,7 @@
 !21 = !DILocation(line: 14, column: 1, scope: !15)
 !22 = !DILocation(line: 16, column: 1, scope: !15)
 !23 = !{!24}
-!24 = !{i64 2, i64 -1, i64 -1, i1 true}
+!24 = !{i64 0, i64 2, i64 -1, i64 -1, i1 true}
 !25 = distinct !DISubprogram(name: ".omp_outlined.", scope: !1, file: !1, line: 9, type: !26, scopeLine: 9, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !33)
 !26 = !DISubroutineType(types: !27)
 !27 = !{null, !28, !28}
Index: llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
===
--- llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
+++ llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
@@ -9,7 +9,7 @@
 ; CHECK:   CS calls function 'd'
 ;
 ; CHECK: Call graph node for function: '.omp_outlined..0'<<{{.*}}>>  #uses=1
-; 
+;
 ; CHECK: Call graph node for function: '.omp_outlined..1'<<{{.*}}>>  #uses=3
 ; CHECK:   CS<{{.*}}> calls function 'd'
 ;
@@ -89,4 +89,4 @@
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{!"clang version 11.0.0"}
 !2 = !{!3}
-!3 = !{i64 2, i64 -1, i64 -1, i1 true}
+!3 = !{i64 0, i64 2, i64 -1, i64 -1, i1 true}
Index: llvm/test/Transforms/OpenMP/parallel_deletion.ll
===
--- llvm/test/Transforms/OpenMP/parallel_deletion.ll
+++ llvm/test/Transforms/OpenMP/

[PATCH] D98134: [WIP][RFC] Introduce callback argument encoding mode into callback metadata

2021-03-08 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 updated this revision to Diff 329163.
tianshilei1992 added a comment.

update doc in clang


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98134

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-callback.c
  clang/test/CodeGen/callback_annotated.c
  clang/test/CodeGen/callback_openmp.c
  clang/test/CodeGenCXX/attr-callback.cpp
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/Sema/attr-callback-broken.c
  clang/test/Sema/attr-callback.c
  clang/test/SemaCXX/attr-callback.cpp
  llvm/include/llvm/IR/AbstractCallSite.h
  llvm/include/llvm/IR/MDBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/IR/AbstractCallSite.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/test/Analysis/CallGraph/callback-calls.ll
  llvm/test/Analysis/CallGraph/ignore-callback-uses.ll
  llvm/test/Transforms/Attributor/IPConstantProp/multiple_callbacks.ll
  llvm/test/Transforms/Attributor/IPConstantProp/openmp_parallel_for.ll
  llvm/test/Transforms/Attributor/IPConstantProp/pthreads.ll
  llvm/test/Transforms/Attributor/IPConstantProp/thread_local_acs.ll
  llvm/test/Transforms/Attributor/callbacks.ll
  llvm/test/Transforms/Attributor/noundef.ll
  llvm/test/Transforms/OpenMP/parallel_deletion.ll
  llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
  llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
  llvm/test/Transforms/OpenMP/parallel_region_merging.ll
  llvm/unittests/IR/AbstractCallSiteTest.cpp
  llvm/unittests/IR/LegacyPassManagerTest.cpp

Index: llvm/unittests/IR/LegacyPassManagerTest.cpp
===
--- llvm/unittests/IR/LegacyPassManagerTest.cpp
+++ llvm/unittests/IR/LegacyPassManagerTest.cpp
@@ -768,7 +768,7 @@
"}\n"
"\n"
"!0 = !{!1}\n"
-   "!1 = !{i64 0, i64 1, i1 false}";
+   "!1 = !{i64 0, i64 0, i64 1, i1 false}";
 
   SMDiagnostic Err;
   std::unique_ptr M = parseAssemblyString(IR, Err, Context);
Index: llvm/unittests/IR/AbstractCallSiteTest.cpp
===
--- llvm/unittests/IR/AbstractCallSiteTest.cpp
+++ llvm/unittests/IR/AbstractCallSiteTest.cpp
@@ -36,7 +36,7 @@
   "}\n"
   "declare !callback !0 void @broker(i32, void (i8*, ...)*, ...)\n"
   "!0 = !{!1}\n"
-  "!1 = !{i64 1, i64 -1, i1 true}";
+  "!1 = !{i64 0, i64 1, i64 -1, i1 true}";
 
   std::unique_ptr M = parseIR(C, IR);
   ASSERT_TRUE(M);
Index: llvm/test/Transforms/OpenMP/parallel_region_merging.ll
===
--- llvm/test/Transforms/OpenMP/parallel_region_merging.ll
+++ llvm/test/Transforms/OpenMP/parallel_region_merging.ll
@@ -786,7 +786,7 @@
 
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{!2}
-!2 = !{i64 2, i64 -1, i64 -1, i1 true}
+!2 = !{i64 0, i64 2, i64 -1, i64 -1, i1 true}
 ; CHECK-LABEL: define {{[^@]+}}@merge
 ; CHECK-SAME: (i32 [[A:%.*]]) local_unnamed_addr {
 ; CHECK-NEXT:  entry:
Index: llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
===
--- llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
+++ llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
@@ -96,7 +96,7 @@
 !21 = !DILocation(line: 14, column: 1, scope: !15)
 !22 = !DILocation(line: 16, column: 1, scope: !15)
 !23 = !{!24}
-!24 = !{i64 2, i64 -1, i64 -1, i1 true}
+!24 = !{i64 0, i64 2, i64 -1, i64 -1, i1 true}
 !25 = distinct !DISubprogram(name: ".omp_outlined.", scope: !1, file: !1, line: 9, type: !26, scopeLine: 9, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !33)
 !26 = !DISubroutineType(types: !27)
 !27 = !{null, !28, !28}
Index: llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
===
--- llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
+++ llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
@@ -9,7 +9,7 @@
 ; CHECK:   CS calls function 'd'
 ;
 ; CHECK: Call graph node for function: '.omp_outlined..0'<<{{.*}}>>  #uses=1
-; 
+;
 ; CHECK: Call graph node for function: '.omp_outlined..1'<<{{.*}}>>  #uses=3
 ; CHECK:   CS<{{.*}}> calls function 'd'
 ;
@@ -89,4 +89,4 @@
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{!"clang version 11.0.0"}
 !2 = !{!3}
-!3 = !{i64 2, i64 -1, i64 -1, i1 true}
+!3 = !{i64 0, i64 2, i64 -1, i64 -1, i1 true}
Index: llvm/test/Transforms/OpenMP/parallel_deletion.ll
===
--- llvm/test/Transforms/OpenMP/parallel_deletion.ll
+++ llvm/test/Transforms/OpenMP/parallel_deletion.ll

[PATCH] D98134: [WIP][RFC] Introduce callback argument encoding mode into callback metadata

2021-03-06 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3679
 S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments)
-<< AL << (unsigned)(EncodingIndices.size() - 1);
+<< AL << (unsigned)(CalleeFnProtoType->getNumParams() + 2);
 return;

Need to revise this line to see which is the right number passed here.



Comment at: llvm/include/llvm/IR/AbstractCallSite.h:55
   struct CallbackInfo {
+enum ParameterEncodingModeTy : int64_t {
+  FLATTEN = 0,

I guess this enum should be in a more common place such that other files can 
use it. Any suggestion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98134

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


[PATCH] D98134: [WIP][RFC] Introduce callback argument encoding mode into callback metadata

2021-03-06 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 created this revision.
Herald added subscribers: dexonsmith, okura, kuter, hiraditya.
Herald added a reviewer: aaron.ballman.
tianshilei1992 requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added subscribers: llvm-commits, cfe-commits, bbn, sstefan1.
Herald added a reviewer: baziotis.
Herald added projects: clang, LLVM.

Current `!callback` metadata assumes that arguments of callback function
are encoded in broker function arguments, as the following example shows:

  void callback(int, float, double);
  void broker(void(*cb)(int, float, double), int, float, double);

We call the argument encoding as "flatten mode". However, it is not always the
case. For some programming model, such as CUDA and OpenMP offloading, arguments
are "stacked":

  void callback(int *, float *, double *);
  void broker(void(*cb)(int *, float *, double *), void **args, size_t size) {
// ...
cb(args[0], args[1], args[2]);
  }

Let's call this encode as "stacked mode" (a more approporiate name is welcomed).
Current `!callback` metadata cannot handle this case very well. In fact, we can
establish connection between callback function arguments and `args` passed to
the broker function via the use chain of `args`. For example, before we call the
broker function, we need to construct `args` by setting its every element. In
this way, we can know which pointer eventually corresponds to which callback
function argument.

In order to do that, we need to tell apart the two different parameter encoding
modes. In this patch, the `!callback` metadata is updated in the following way:

1. The first operand is an `i64` value, representing parameter encoding mode. 0 
is "flatten mode", and 1 is "stacked mode".
2. The second operand is the callback function index, same as the previous 
first operand.
3. The next one or more operands are still parameter encoding with a little 
difference. If the encoding mode is 0, it is same as current way. If the mode 
is 1, there must only be two operands: one is the index of the base pointer 
(e.g. the index of `void **args` above), and the second is the index of its 
size (e.g. the index of `size_t size` above).
4. The last operand is still for the variadic arguments.

The 3rd pointer above is not covered by this patch for now. It's probably worth
another patch. This patch only add the support for encoding mode, so an extra
`i64 0` should be added everywhere, and for now it can only be 0. The update of
documents will be covered by this patch as well, which will be done soon.

P.S. People might argue in CUDA or OpenMP offloading, the broker function
actually doesn't accept a callback function pointer. It's usually a global
symbol which can be used to find the entry kernel function during the runtime.
More important, the kernel function (or "callback" function) cannot be seen in
the host module at all. What's the point of doing this? Yes, that's true for 
now.
We're working on the heterogenous module, which basically will "merge" (or 
"link")
kernel moduel and host module together. In this way, we can optimize them with
the information from both host and kernel size. We can bridge the kernel 
function
and broker function via the global symbol somehow to make the kernel function a
"virtual" callback function. Although pointers in `void **args` will not be used
by the kernel function directly (as shown in the example above), the real
arguments passed to the kernel function are 1:1 mapped from pointers in `args`.
Therefore the correspondence still exists.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98134

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-callback.c
  clang/test/CodeGen/callback_annotated.c
  clang/test/CodeGen/callback_openmp.c
  clang/test/CodeGenCXX/attr-callback.cpp
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/Sema/attr-callback-broken.c
  clang/test/Sema/attr-callback.c
  clang/test/SemaCXX/attr-callback.cpp
  llvm/include/llvm/IR/AbstractCallSite.h
  llvm/include/llvm/IR/MDBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/IR/AbstractCallSite.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/test/Analysis/CallGraph/callback-calls.ll
  llvm/test/Analysis/CallGraph/ignore-callback-uses.ll
  llvm/test/Transforms/Attributor/IPConstantProp/multiple_callbacks.ll
  llvm/test/Transforms/Attributor/IPConstantProp/openmp_parallel_for.ll
  llvm/test/Transforms/Attributor/IPConstantProp/pthreads.ll
  llvm/test/Transforms/Attributor/IPConstantProp/thread_local_acs.ll
  llvm/test/Transforms/Attributor/callbacks.ll
  llvm/test/Transforms/Attributor/noundef.ll
  llvm/test/Transforms/OpenMP/parallel_deletion.ll
  llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
  llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
  llvm/test/Transforms/Op