[PATCH] D148370: [Clang][Flang][OpenMP] Add loadOffloadInfoMetadata and createOffloadEntriesAndInfoMetadata into OMPIRBuilder's finalize and initialize

agozillon added a comment. Thank you very much for the quick response time on the review and the review @jdoerfert! I believe I have applied all of your current feedback in the last update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148370/new/

[PATCH] D148370: [Clang][Flang][OpenMP] Add loadOffloadInfoMetadata and createOffloadEntriesAndInfoMetadata into OMPIRBuilder's finalize and initialize

agozillon updated this revision to Diff 519048. agozillon added a comment. - Move hostIRFilePath initialize invocation to ModuleTranslation.cpp to respect TargetOp patch - Apply reviewer feedback - Tidy up missed style guidelines i sillily forgot about Repository: rG LLVM Github Monorepo CHA

[PATCH] D148370: [Clang][Flang][OpenMP] Add loadOffloadInfoMetadata and createOffloadEntriesAndInfoMetadata into OMPIRBuilder's finalize and initialize

agozillon updated this revision to Diff 519050. agozillon marked an inline comment as done. agozillon added a comment. - Remove unneccessary OMPIRBuilder scope as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148370/new/ https://reviews.llvm.o

[PATCH] D148370: [Clang][Flang][OpenMP] Add loadOffloadInfoMetadata and createOffloadEntriesAndInfoMetadata into OMPIRBuilder's finalize and initialize

agozillon added a comment. Updated to fix the style issues that were present and pointed out by @jdoerfert thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148370/new/ https://reviews.llvm.org/D148370 _

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

agozillon added a comment. Thank you for the review @jsjodin I'll wait a couple of days to see if @jdoerfert (or anyone else) has any further input before I land the patch! I will apply the nit before landing the patch, so the final reference commit in Phabricator will reflect it. Repository:

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

agozillon added a comment. Thank you both very much, I'll push this up tomorrow then when I'm better able to babysit the buildbot in-case of any CI issues as it's getting a little late in the EU (provided no one else has issues with the code of course between now and then)! Repository: rG L

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

This revision was automatically updated to reflect the committed changes. Closed by commit rGcda46cc4f921: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable &… (authored by agozillon). Changed prior to commit: https://reviews.llvm.org/D149162?vs=524279&id=529258#toc Repository: rG

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

agozillon added a comment. Thank you very much! I'm aware and currently trying to reproduce it locally and fix the issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149162/new/ https://reviews.llvm.org/D149162 _

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

agozillon added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:5201-5204 + if (auto EC = sys::fs::getUniqueID(FileName, ID)) { +assert(EC && + "Unable to get unique ID for file, during getTargetEntryUniqueInfo"); + } fmaye

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

agozillon added a comment. I have hopefully fixed the sanitizer issue (the incorrect assert, thank you again for the catch) with the following patch: https://github.com/llvm/llvm-project/commit/309023263dba3b02bc885101faa47d110f662f99 it was a slightly more involved change than I expected, I ha

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

agozillon added a comment. In D149162#4404379 , @fmayer wrote: > In D149162#4404271 , @agozillon > wrote: > >> I have hopefully fixed the sanitizer issue (the incorrect assert, thank you >> again for the catch)

<    1   2