[Openmp-commits] [PATCH] D72525: [LIBOMPTARGET] Do not increment/decrement the refcount for "declare target" objects

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added a comment.

Change looks good to me as is. Some notes / questions on the reference counting.

We may regret using long instead of int64_t, if this is used on x86-64 with 32 bit long. E.g. windows. Are we sure a 32 bit counter can't overflow in pathological cases?

Is the signed counter type a performance optimization to hit the UB on overflow? Expected it to be unsigned as the value is never negative (iiuc).

An alternative to the bool is to use ~0 to indicate infinite count. That works on the basis that we don't increment the recount if it's inf, so no overflow, and that recounts can't reach uint64_max on a nominally 64 bit address space.

The bool/long or sentinel value are probably subtle enough to warrant a class recount, which likely already exists in llvm somewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72525