jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.
LGTM, one comment and two nits. Feel free to commit if you agree with the
suggestions or come back with concerns. Thanks for working on this and taking
the time to fix it like this!
================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:124
+ /// a pointer and can thus be passed directly, or a pointer to
+ /// \p Original otherwise.
/// \param ReplVal The replacement value, thus a copy or new created version
----------------
It is not a pointer to original, is it? It is an equivalent but potentially not
the same IR value.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:740
+ Builder.restoreIP(PrivCB(InnerAllocaIP, Builder.saveIP(), V,
+ (Reloaded ? *Reloaded : V), ReplacementValue));
assert(ReplacementValue &&
----------------
Nit: Rename `Reloaded` to `Inner` to make it consistent with the callback
(description). With `Value* Inner = V;` you can avoid the select above.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:745
- for (Use *UPtr : Uses)
- UPtr->set(ReplacementValue);
+ if (ReplacementValue != &V)
+ for (Use *UPtr : Uses)
----------------
Nit: This could go back up again I guess.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92189/new/
https://reviews.llvm.org/D92189
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
