-
Notifications
You must be signed in to change notification settings - Fork 802
Generate an opt report of kernel arguments. #3492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 26 commits
d99d3a7
4845eb1
8b5d07c
e69616e
5d12df4
c44d670
87ce263
232a8a9
4be5deb
978d6b8
1db1025
82cacc9
5254539
32454ac
d316062
95b6c9f
aeed807
1287604
8737bc1
c5d2ae3
6ce7268
f33353d
ddad6f9
0af4422
2c0a35c
f99e200
0937042
6a87209
eac6f25
c04374c
84a2cee
8efb79a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,57 @@ | ||||||
| //===---------------------- SyclOptReport.h ---------------------*- C++ -*-===// | ||||||
| // | ||||||
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||||||
| // See https://llvm.org/LICENSE.txt for license information. | ||||||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||||||
| // | ||||||
| //===----------------------------------------------------------------------===// | ||||||
| /// | ||||||
| /// \file | ||||||
| /// Defines clang::SyclOptReport class. | ||||||
|
||||||
| /// | ||||||
| //===----------------------------------------------------------------------===// | ||||||
|
|
||||||
| #ifndef LLVM_CLANG_BASIC_SYCLOPTREPORTHANDLER_H | ||||||
| #define LLVM_CLANG_BASIC_SYCLOPTREPORTHANDLER_H | ||||||
|
|
||||||
| #include "clang/Basic/SourceLocation.h" | ||||||
| #include "llvm/ADT/DenseMap.h" | ||||||
| #include "llvm/ADT/StringRef.h" | ||||||
|
|
||||||
| namespace clang { | ||||||
|
|
||||||
| class FunctionDecl; | ||||||
|
|
||||||
| class SyclOptReportHandler { | ||||||
| private: | ||||||
| struct OptReportInfo { | ||||||
| std::string KernelArgName; | ||||||
| std::string KernelArgType; | ||||||
| SourceLocation KernelArgLoc; | ||||||
|
|
||||||
| OptReportInfo(std::string ArgName, std::string ArgType, | ||||||
| SourceLocation ArgLoc) | ||||||
| : KernelArgName(ArgName), KernelArgType(ArgType), KernelArgLoc(ArgLoc) { | ||||||
|
||||||
| : KernelArgName(ArgName), KernelArgType(ArgType), KernelArgLoc(ArgLoc) { | |
| : KernelArgName(std::move(ArgName)), KernelArgType(std::move(ArgType)), KernelArgLoc(ArgLoc) { |
Might as well use the move constructors.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than Has and Get that do very similar things, how about: Optional<SmallVectorImpl<OptReportInfo>&> (Note, I can never remember whether LLVM's Optional allows you to wrap a reference, so if that doesn't work, then feel free to ignore the suggestion.)
This would allow callers to call GetInfo() and rather than getting an assertion if they forgot to call HasOptReportInfo(), they get llvm::None instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That didn't work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bummer! Thanks for trying!
Then I think the only other suggestion I have is to return a SmallVectorImpl<OptReportInfo> instead of the concrete SmallVector<> type, if possible. This way, callers can decide for themselves what constitutes "small" and aren't stuck agreeing that 4 is it.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -304,28 +304,12 @@ namespace clang { | |||||||||||||||||
| return; | ||||||||||||||||||
|
|
||||||||||||||||||
| LLVMContext &Ctx = getModule()->getContext(); | ||||||||||||||||||
|
|
||||||||||||||||||
| std::unique_ptr<DiagnosticHandler> OldDiagnosticHandler = | ||||||||||||||||||
| Ctx.getDiagnosticHandler(); | ||||||||||||||||||
| Ctx.setDiagnosticHandler(std::make_unique<ClangDiagnosticHandler>( | ||||||||||||||||||
| CodeGenOpts, this)); | ||||||||||||||||||
|
|
||||||||||||||||||
| Expected<std::unique_ptr<llvm::ToolOutputFile>> OptRecordFileOrErr = | ||||||||||||||||||
| setupLLVMOptimizationRemarks( | ||||||||||||||||||
| Ctx, CodeGenOpts.OptRecordFile, CodeGenOpts.OptRecordPasses, | ||||||||||||||||||
| CodeGenOpts.OptRecordFormat, CodeGenOpts.DiagnosticsWithHotness, | ||||||||||||||||||
| CodeGenOpts.DiagnosticsHotnessThreshold); | ||||||||||||||||||
|
|
||||||||||||||||||
| if (Error E = OptRecordFileOrErr.takeError()) { | ||||||||||||||||||
| reportOptRecordError(std::move(E), Diags, CodeGenOpts); | ||||||||||||||||||
| return; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| std::unique_ptr<llvm::ToolOutputFile> OptRecordFile = | ||||||||||||||||||
| std::move(*OptRecordFileOrErr); | ||||||||||||||||||
|
|
||||||||||||||||||
| if (OptRecordFile && | ||||||||||||||||||
| CodeGenOpts.getProfileUse() != CodeGenOptions::ProfileNone) | ||||||||||||||||||
| Ctx.setDiagnosticsHotnessRequested(true); | ||||||||||||||||||
| // The diagnostic handler is now processed in OptRecordFileRAII. | ||||||||||||||||||
|
|
||||||||||||||||||
| // The parallel_for_work_group legalization pass can emit calls to | ||||||||||||||||||
| // builtins function. Definitions of those builtins can be provided in | ||||||||||||||||||
|
|
@@ -349,9 +333,6 @@ namespace clang { | |||||||||||||||||
| getModule(), Action, std::move(AsmOutStream)); | ||||||||||||||||||
|
|
||||||||||||||||||
| Ctx.setDiagnosticHandler(std::move(OldDiagnosticHandler)); | ||||||||||||||||||
|
|
||||||||||||||||||
| if (OptRecordFile) | ||||||||||||||||||
| OptRecordFile->keep(); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| void HandleTagDeclDefinition(TagDecl *D) override { | ||||||||||||||||||
|
|
@@ -1046,8 +1027,52 @@ CodeGenAction::loadModule(MemoryBufferRef MBRef) { | |||||||||||||||||
| return {}; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| namespace { | ||||||||||||||||||
| // Handles the initialization and cleanup of the OptRecordFile. This | ||||||||||||||||||
| // customization allows initialization before the clang codegen runs | ||||||||||||||||||
| // so it can also emit to the opt report. | ||||||||||||||||||
| struct OptRecordFileRAII { | ||||||||||||||||||
| std::unique_ptr<llvm::ToolOutputFile> OptRecordFile; | ||||||||||||||||||
| std::unique_ptr<DiagnosticHandler> OldDiagnosticHandler; | ||||||||||||||||||
| llvm::LLVMContext &Ctx; | ||||||||||||||||||
|
|
||||||||||||||||||
| OptRecordFileRAII(CodeGenAction &CGA, llvm::LLVMContext &Ctx, | ||||||||||||||||||
| BackendConsumer &BC) | ||||||||||||||||||
| : Ctx(Ctx) { | ||||||||||||||||||
| CompilerInstance &CI = CGA.getCompilerInstance(); | ||||||||||||||||||
| CodeGenOptions &CodeGenOpts = CI.getCodeGenOpts(); | ||||||||||||||||||
|
|
||||||||||||||||||
| OldDiagnosticHandler = Ctx.getDiagnosticHandler(); | ||||||||||||||||||
|
||||||||||||||||||
| : Ctx(Ctx) { | |
| CompilerInstance &CI = CGA.getCompilerInstance(); | |
| CodeGenOptions &CodeGenOpts = CI.getCodeGenOpts(); | |
| OldDiagnosticHandler = Ctx.getDiagnosticHandler(); | |
| : Ctx(Ctx), OldDiagnosticHandler(Ctx.getDiagnosticHandler()) { | |
| CompilerInstance &CI = CGA.getCompilerInstance(); | |
| CodeGenOptions &CodeGenOpts = CI.getCodeGenOpts(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ | |
| #include "clang/CodeGen/CGFunctionInfo.h" | ||
| #include "clang/Frontend/FrontendDiagnostic.h" | ||
| #include "llvm/ADT/ArrayRef.h" | ||
| #include "llvm/Analysis/OptimizationRemarkEmitter.h" | ||
| #include "llvm/Frontend/OpenMP/OMPIRBuilder.h" | ||
| #include "llvm/IR/DataLayout.h" | ||
| #include "llvm/IR/Dominators.h" | ||
|
|
@@ -1521,6 +1522,27 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn, | |
| // Emit the standard function prologue. | ||
| StartFunction(GD, ResTy, Fn, FnInfo, Args, Loc, BodyRange.getBegin()); | ||
|
|
||
| SyclOptReportHandler &OptReportHandler = | ||
| CGM.getDiags().getSYCLOptReportHandler(); | ||
| if (OptReportHandler.HasOptReportInfo(FD)) { | ||
AaronBallman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| llvm::OptimizationRemarkEmitter ORE(Fn); | ||
| for (auto ORI : llvm::enumerate(OptReportHandler.GetInfo(FD))) { | ||
| llvm::DiagnosticLocation DL = | ||
| SourceLocToDebugLoc(ORI.value().KernelArgLoc); | ||
| llvm::OptimizationRemark Remark("sycl", "Region", DL, | ||
| &Fn->getEntryBlock()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We seem to calling ORI.value() multiple times below; perhaps save ORI.value().KernelArgName in a variable here and the reuse that? |
||
| Remark << "Argument " << llvm::ore::NV("Argument", ORI.index()) | ||
| << " for function kernel: " | ||
| << llvm::ore::NV(ORI.value().KernelArgName.empty() ? "&" : "") | ||
| << " " << Fn->getName() << "." | ||
| << llvm::ore::NV(ORI.value().KernelArgName.empty() | ||
| ? " " | ||
| : ORI.value().KernelArgName) | ||
| << "(" << ORI.value().KernelArgType << ")"; | ||
| ORE.emit(Remark); | ||
| } | ||
| } | ||
|
|
||
| // Save parameters for coroutine function. | ||
| if (Body && isa_and_nonnull<CoroutineBodyStmt>(Body)) | ||
| for (const auto *ParamDecl : FD->parameters()) | ||
|
|
@@ -2623,8 +2645,7 @@ Address CodeGenFunction::EmitIntelFPGAFieldAnnotations(SourceLocation Location, | |
| // llvm.ptr.annotation intrinsic accepts a pointer to integer of any width - | ||
| // don't perform bitcasts if value is integer | ||
| if (VTy->getPointerElementType()->isIntegerTy()) { | ||
| llvm::Function *F = | ||
| CGM.getIntrinsic(llvm::Intrinsic::ptr_annotation, VTy); | ||
| llvm::Function *F = CGM.getIntrinsic(llvm::Intrinsic::ptr_annotation, VTy); | ||
|
||
| V = EmitAnnotationCall(F, V, AnnotStr, Location); | ||
|
|
||
| return Address(V, Addr.getAlignment()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| // RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -fsycl-is-device \ | ||
| // RUN: -Wno-sycl-2017-compat -emit-llvm-bc %s -o %t-host.bc -opt-record-file %t-host.yaml | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you expand this test to check for basic things in the opt-report file? I would like to see that to understand the changes in CodeGenFunction better. |
||
| // RUN: FileCheck -check-prefix=CHECK --input-file %t-host.yaml %s | ||
| // The test generates remarks about the kernel argument, their location and type | ||
| // in the resulting yaml file. | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also add a comment here describing what the test is testing for. |
||
| #include "Inputs/sycl.hpp" | ||
|
|
||
| class second_base { | ||
| public: | ||
| int *e; | ||
| }; | ||
|
|
||
| class InnerFieldBase { | ||
| public: | ||
| int d; | ||
| }; | ||
| class InnerField : public InnerFieldBase { | ||
| int c; | ||
| }; | ||
|
|
||
| struct base { | ||
| public: | ||
| int b; | ||
| InnerField obj; | ||
| }; | ||
|
|
||
| //CHECK: --- !Passed | ||
| //CHECK: Pass:{{.*}}sycl | ||
| //CHECK: Name:{{.*}}Region | ||
| //CHECK: DebugLoc:{{.*}} { File: '{{.*}}kernel-arg-opt-report.cpp', | ||
| //CHECK: Line: 85, Column: 18 } | ||
| //CHECK: Function: _ZTS7derived | ||
| //CHECK: Args: | ||
| //CHECK-NEXT: String: 'Argument ' | ||
| //CHECK-NEXT: Argument: '0' | ||
| //CHECK-NEXT: String: ' for function kernel: ' | ||
| //CHECK-NEXT: String: '&' | ||
| //CHECK-NEXT: String: ' ' | ||
| //CHECK-NEXT: String: _ZTS7derived | ||
| //CHECK-NEXT: String: . | ||
| //CHECK-NEXT: String: ' ' | ||
| //CHECK-NEXT: String: '(' | ||
| //CHECK-NEXT: String: struct base | ||
| //CHECK-NEXT: String: ')' | ||
|
|
||
| //CHECK: --- !Passed | ||
| //CHECK: Pass:{{.*}}sycl | ||
| //CHECK: Name:{{.*}}Region | ||
| //CHECK: DebugLoc:{{.*}} { File: '{{.*}}kernel-arg-opt-report.cpp', | ||
| //CHECK: Line: 11, Column: 8 } | ||
| //CHECK: Function: _ZTS7derived | ||
| //CHECK: Args: | ||
| //CHECK-NEXT: String: 'Argument ' | ||
| //CHECK-NEXT: Argument: '1' | ||
| //CHECK-NEXT: String: ' for function kernel: ' | ||
| //CHECK-NEXT: String: '' | ||
| //CHECK-NEXT: String: ' ' | ||
| //CHECK-NEXT: String: _ZTS7derived | ||
| //CHECK-NEXT: String: . | ||
| //CHECK-NEXT: String: e | ||
| //CHECK-NEXT: String: '(' | ||
| //CHECK-NEXT: String: struct __wrapper_class | ||
| //CHECK-NEXT: String: ')' | ||
|
|
||
| //CHECK: --- !Passed | ||
| //CHECK: Pass:{{.*}}sycl | ||
| //CHECK: Name:{{.*}}Region | ||
| //CHECK: DebugLoc:{{.*}} { File: '{{.*}}kernel-arg-opt-report.cpp', | ||
| //CHECK: Line: 86, Column: 7 } | ||
| //CHECK: Function: _ZTS7derived | ||
| //CHECK: Args: | ||
| //CHECK-NEXT: String: 'Argument ' | ||
| //CHECK-NEXT: Argument: '2' | ||
| //CHECK-NEXT: String: ' for function kernel: ' | ||
| //CHECK-NEXT: String: '' | ||
| //CHECK-NEXT: String: ' ' | ||
| //CHECK-NEXT: String: _ZTS7derived | ||
| //CHECK-NEXT: String: . | ||
| //CHECK-NEXT: String: a | ||
| //CHECK-NEXT: String: '(' | ||
| //CHECK-NEXT: String: int | ||
| //CHECK-NEXT: String: ')' | ||
|
|
||
| struct derived : base, second_base { | ||
| int a; | ||
|
|
||
| void operator()() const { | ||
| } | ||
| }; | ||
|
|
||
| int main() { | ||
| sycl::queue q; | ||
|
|
||
| q.submit([&](cl::sycl::handler &cgh) { | ||
| derived f{}; | ||
| cgh.single_task(f); | ||
| }); | ||
|
|
||
| return 0; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filename is SyclOptReportHandler.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please change this as well?