[llvm] 1caabbe - [OpaquePtr] Fix initialization-order-fiasco

[Nikita Popov via llvm-commits]

On Fri, Nov 5, 2021 at 3:29 AM Vitaly Buka via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

>
> Author: Vitaly Buka
> Date: 2021-11-04T19:29:06-07:00
> New Revision: 1caabbef8e8e73b3dd0cf1f15cf7417d75b7621c
>
> URL:
> https://github.com/llvm/llvm-project/commit/1caabbef8e8e73b3dd0cf1f15cf7417d75b7621c
> DIFF:
> https://github.com/llvm/llvm-project/commit/1caabbef8e8e73b3dd0cf1f15cf7417d75b7621c.diff
>
> LOG: [OpaquePtr] Fix initialization-order-fiasco
>
> Asan detects it after D112732.
>

Can you please explain why this change is needed? (It would have been nice
if that explanation was part of the commit message...)

Why do we now have both enableOpaquePointers() and setOpaquePointers(), as
well as supportsTypedPointers() and getOpaquePointers()? These method pairs
look redundant to me.

Regards,
Nikita

Added:
>
>
> Modified:
>     llvm/lib/IR/LLVMContext.cpp
>     llvm/lib/IR/LLVMContextImpl.cpp
>     llvm/lib/IR/LLVMContextImpl.h
>     llvm/lib/IR/Type.cpp
>
> Removed:
>
>
>
>
> ################################################################################
> diff  --git a/llvm/lib/IR/LLVMContext.cpp b/llvm/lib/IR/LLVMContext.cpp
> index dce5d17c9eea..90716d9c81a6 100644
> --- a/llvm/lib/IR/LLVMContext.cpp
> +++ b/llvm/lib/IR/LLVMContext.cpp
> @@ -351,9 +351,9 @@ std::unique_ptr<DiagnosticHandler>
> LLVMContext::getDiagnosticHandler() {
>  void LLVMContext::enableOpaquePointers() const {
>    assert(pImpl->PointerTypes.empty() && pImpl->ASPointerTypes.empty() &&
>           "Must be called before creating any pointer types");
> -  pImpl->OpaquePointers = true;
> +  pImpl->setOpaquePointers(true);
>  }
>
>  bool LLVMContext::supportsTypedPointers() const {
> -  return !pImpl->OpaquePointers;
> +  return !pImpl->getOpaquePointers();
>  }
>
> diff  --git a/llvm/lib/IR/LLVMContextImpl.cpp
> b/llvm/lib/IR/LLVMContextImpl.cpp
> index 068bc58b6796..ebbf382aea38 100644
> --- a/llvm/lib/IR/LLVMContextImpl.cpp
> +++ b/llvm/lib/IR/LLVMContextImpl.cpp
> @@ -35,8 +35,7 @@ LLVMContextImpl::LLVMContextImpl(LLVMContext &C)
>        X86_FP80Ty(C, Type::X86_FP80TyID), FP128Ty(C, Type::FP128TyID),
>        PPC_FP128Ty(C, Type::PPC_FP128TyID), X86_MMXTy(C,
> Type::X86_MMXTyID),
>        X86_AMXTy(C, Type::X86_AMXTyID), Int1Ty(C, 1), Int8Ty(C, 8),
> -      Int16Ty(C, 16), Int32Ty(C, 32), Int64Ty(C, 64), Int128Ty(C, 128),
> -      OpaquePointers(OpaquePointersCL) {}
> +      Int16Ty(C, 16), Int32Ty(C, 32), Int64Ty(C, 64), Int128Ty(C, 128) {}
>
>  LLVMContextImpl::~LLVMContextImpl() {
>    // NOTE: We need to delete the contents of OwnedModules, but Module's
> dtor
> @@ -233,3 +232,11 @@ OptPassGate &LLVMContextImpl::getOptPassGate() const {
>  void LLVMContextImpl::setOptPassGate(OptPassGate& OPG) {
>    this->OPG = &OPG;
>  }
> +
> +bool LLVMContextImpl::getOpaquePointers() {
> +  if (LLVM_UNLIKELY(!(OpaquePointers.hasValue())))
> +    OpaquePointers = OpaquePointersCL;
> +  return *OpaquePointers;
> +}
> +
> +void LLVMContextImpl::setOpaquePointers(bool OP) { OpaquePointers = OP; }
>
> diff  --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h
> index b17f581faaa6..d84714d9b1f1 100644
> --- a/llvm/lib/IR/LLVMContextImpl.h
> +++ b/llvm/lib/IR/LLVMContextImpl.h
> @@ -1461,10 +1461,7 @@ class LLVMContextImpl {
>    unsigned NamedStructTypesUniqueID = 0;
>
>    DenseMap<std::pair<Type *, uint64_t>, ArrayType*> ArrayTypes;
> -  DenseMap<std::pair<Type *, ElementCount>, VectorType*> VectorTypes;
> -  // TODO: clean up the following after we no longer support non-opaque
> pointer
> -  // types.
> -  bool OpaquePointers;
> +  DenseMap<std::pair<Type *, ElementCount>, VectorType *> VectorTypes;
>    DenseMap<Type*, PointerType*> PointerTypes;  // Pointers in AddrSpace =
> 0
>    DenseMap<std::pair<Type*, unsigned>, PointerType*> ASPointerTypes;
>
> @@ -1544,6 +1541,14 @@ class LLVMContextImpl {
>    /// The lifetime of the object must be guaranteed to extend as long as
> the
>    /// LLVMContext is used by compilation.
>    void setOptPassGate(OptPassGate&);
> +
> +  // TODO: clean up the following after we no longer support non-opaque
> pointer
> +  // types.
> +  bool getOpaquePointers();
> +  void setOpaquePointers(bool OP);
> +
> +private:
> +  Optional<bool> OpaquePointers;
>  };
>
>  } // end namespace llvm
>
> diff  --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp
> index 0a28a001ef0d..d59d87ad631b 100644
> --- a/llvm/lib/IR/Type.cpp
> +++ b/llvm/lib/IR/Type.cpp
> @@ -733,7 +733,7 @@ PointerType *PointerType::get(Type *EltTy, unsigned
> AddressSpace) {
>    LLVMContextImpl *CImpl = EltTy->getContext().pImpl;
>
>    // Automatically convert typed pointers to opaque pointers.
> -  if (CImpl->OpaquePointers)
> +  if (CImpl->getOpaquePointers())
>      return get(EltTy->getContext(), AddressSpace);
>
>    // Since AddressSpace #0 is the common case, we special case it.
> @@ -747,7 +747,7 @@ PointerType *PointerType::get(Type *EltTy, unsigned
> AddressSpace) {
>
>  PointerType *PointerType::get(LLVMContext &C, unsigned AddressSpace) {
>    LLVMContextImpl *CImpl = C.pImpl;
> -  assert(CImpl->OpaquePointers &&
> +  assert(CImpl->getOpaquePointers() &&
>           "Can only create opaque pointers in opaque pointer mode");
>
>    // Since AddressSpace #0 is the common case, we special case it.
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211105/6fa8b9ad/attachment.html>