<div dir="ltr">Hello Ruiling,<div> </div><div>Manasi will be taking over this patch for me. Thanks for your review!</div><div> </div><div>Laura</div></div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Jul 29, 2015 at 12:19 AM, Song, Ruiling <<a href="mailto:ruiling.song@intel.com" target="_blank">ruiling.song@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div lang="EN-US" link="blue" vlink="purple"> <div> Hi Laura, I see! Thanks for the info. The patch basically looks good, but it does not work under llvm 3.6. Llvm 3.6 change the raw_fd_ostream constructor. It is defined as: raw_fd_ostream( StringRef fileName, std::error_code &EC, sys::fs::OpenFlags Flags) So, you need to add some logic like below to handle this: #if LLVM_VERSION_MAJOR == 3 && LLVM_VERSION_MINOR < 6 std::string err; llvm::raw_fd_ostream ostream (dumpLLVMFileName.c_str(), err, llvm::sys::fs::F_RW); if (err.empty()) { out_module->print(ostream, 0); } //Otherwise, you'll have to make do without the dump. #else std::error_code err; llvm::raw_fd_ostream ostream (dumpLLVMFileName.c_str(), err, llvm::sys::fs::F_RW); if (!err) { out_module->print(ostream, 0); } #endif <a name="14ed8b0975bd5ad0__MailEndCompose"> </a> Other parts of the patch LGTM. So could you fix it and send a new version? And also please include “Signed-off-by: ” in your patch. Thanks! Ruiling <div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt"> <div> <div style="border:none;border-top:solid #e1e1e1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Beignet [mailto:<a href="mailto:beignet-bounces@lists.freedesktop.org" target="_blank">beignet-bounces@lists.freedesktop.org</a>] On Behalf Of Laura Ekstrand Sent: Wednesday, July 29, 2015 5:03 AM To: Song, Ruiling Cc: Ekstrand, Laura D; <a href="mailto:beignet@lists.freedesktop.org" target="_blank">beignet@lists.freedesktop.org</a> Subject: Re: [Beignet] [PATCH] backend/src/backend: Handle -dump-opt-llvm=[PATH] </div> </div><div><div class="h5"> <div> <div> This is to enable a feature in the Intel tool OpenCL Code Builder. It is a non-standard option. </div> Laura </div> <div> <div> On Tue, Jul 28, 2015 at 12:30 AM, Song, Ruiling <<a href="mailto:ruiling.song@intel.com" target="_blank">ruiling.song@intel.com</a>> wrote: <blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in"> Could you explain why this is needed? And is "dump-opt-llvm" a llvm-standard option or opencl option? Thanks! Ruiling > -----Original Message----- > From: Beignet [mailto:<a href="mailto:beignet-bounces@lists.freedesktop.org" target="_blank">beignet-bounces@lists.freedesktop.org</a>] On Behalf Of > Laura Ekstrand > Sent: Friday, July 10, 2015 3:04 AM > To: <a href="mailto:beignet@lists.freedesktop.org" target="_blank">beignet@lists.freedesktop.org</a> > Cc: Ekstrand, Laura D > Subject: [Beignet] [PATCH] backend/src/backend: Handle -dump-opt-llvm=[PATH] > <div> <div> > Allows the user to request a dump of the LLVM-generated IR to the file specified > in [PATH]. > --- > backend/src/backend/program.cpp | 26 +++++++++++++++++++++++--- > 1 file changed, 23 insertions(+), 3 deletions(-) > > diff --git a/backend/src/backend/program.cpp > b/backend/src/backend/program.cpp index e4cdeaa..b55f75a 100644 > --- a/backend/src/backend/program.cpp > +++ b/backend/src/backend/program.cpp > @@ -640,6 +640,7 @@ namespace gbe { > const char *options, > const char *temp_header_path, > std::vector<std::string>& clOpt, > + std::string& dumpLLVMFileName, > std::string& clName, > int& optLevel, > size_t stringSize, @@ -719,6 +720,11 @@ namespace gbe { > clOpt.push_back("__FAST_RELAXED_MATH__=1"); > } > > + if(str.find("-dump-opt-llvm=") != std::string::npos) { > + dumpLLVMFileName = str.substr(str.find("=") + 1); > + continue; // Don't push this str back; ignore it. > + } > + > clOpt.push_back(str); > } > free(str); > @@ -781,8 +787,10 @@ namespace gbe { > int optLevel = 1; > std::vector<std::string> clOpt; > std::string clName; > - if (!processSourceAndOption(source, options, NULL, clOpt, clName, > - optLevel, stringSize, err, errSize)) > + std::string dumpLLVMFileName; > + if (!processSourceAndOption(source, options, NULL, clOpt, > + dumpLLVMFileName, clName, optLevel, > + stringSize, err, errSize)) > return NULL; > > gbe_program p; > @@ -804,6 +812,16 @@ namespace gbe { > clangErrSize = *errSize; > } > > + // Dump the LLVM if requested. > + if (!dumpLLVMFileName.empty()) { > + std::string err; > + llvm::raw_fd_ostream ostream (dumpLLVMFileName.c_str(), > + err, llvm::sys::fs::F_RW); > + if (err.empty()) { > + out_module->print(ostream, 0); > + } //Otherwise, you'll have to make do without the dump. > + } > + > p = gbe_program_new_from_llvm(deviceID, NULL, out_module, llvm_ctx, > stringSize, > err, errSize, optLevel); > if (err != NULL) > @@ -834,7 +852,9 @@ namespace gbe { > int optLevel = 1; > std::vector<std::string> clOpt; > std::string clName; > - if (!processSourceAndOption(source, options, temp_header_path, clOpt, > clName, > + std::string dumpLLVMFileName; > + if (!processSourceAndOption(source, options, temp_header_path, clOpt, > + dumpLLVMFileName, clName, > optLevel, stringSize, err, errSize)) > return NULL; > > -- > 2.1.0 > > _______________________________________________ > Beignet mailing list > <a href="mailto:Beignet@lists.freedesktop.org" target="_blank">Beignet@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/beignet" target="_blank"> http://lists.freedesktop.org/mailman/listinfo/beignet</a> _______________________________________________ Beignet mailing list <a href="mailto:Beignet@lists.freedesktop.org" target="_blank">Beignet@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/beignet" target="_blank">http://lists.freedesktop.org/mailman/listinfo/beignet</a> </div> </div> </blockquote> </div> </div> </div></div></div> </div> </div> </blockquote></div> </div>