[Openmp-commits] [PATCH] D74925: [OpenMP][Opt] Combine `struct ident_t*` during deduplication

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added a comment.



In D74925#1889185 <https://reviews.llvm.org/D74925#1889185>, @JonChesterfield wrote:

> Cool. Can we reasonably add the reproducible from 44893 to a regression suite, in addition to these IR tests? It's written in C so would need to be under clang's tests.


Done.

> This looks like a good fix for the reported bug. I don't see why this failure mode would be unique to ident_t though, at least from re-reading deduplicateRuntimeCalls - is it a feature of the current set of runtime calls that can be deduplicated?

Good catch. Added enough code to be future proof wrt. miscompiles. Also added an extra test in IR.



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:191
 
-private:
+  /// Return the insertion point used by the underlying IRBuilder.
+  InsertPointTy getInsertionPoint() { return Builder.saveIP(); }
----------------
JonChesterfield wrote:
> Probably don't want to drop the `private` annotation here
I actually do. The new and some of the below functions should be exposed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74925/new/

https://reviews.llvm.org/D74925