[PATCH] D98134: [WIP][RFC] Introduce callback argument encoding mode into callback metadata
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
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
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
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