[PATCH] D108642: [CGProfile] allows undef in metadata node storing function pointers

[Yuanfang Chen via Phabricator via llvm-commits]

ychen added a comment.

In D108642#2962903 <https://reviews.llvm.org/D108642#2962903>, @dexonsmith wrote:

> In D108642#2962880 <https://reviews.llvm.org/D108642#2962880>, @dexonsmith wrote:
>
>> It used to be that when GlobalValue-derived Constants were removed, then metadata would point at `null` instead. Not sure if that changed at some point, or if this is a unique code path, but it's not clear whether having two possible "no value" values (undef vs. null) is a good idea.
>>
>> Can you explain why https://reviews.llvm.org/rGe5d958c45629ccd2f5b5f7432756be1d0fcf052c used `undef` instead of `null`? Should it use `null` instead?
>
> Ah, that was the commit that changed it. It used to hit this logic:
>
>   Value::~Value() {
>     // Notify all ValueHandles (if present) that this value is going away.
>     if (HasValueHandle)
>       ValueHandleBase::ValueIsDeleted(this);
>     if (isUsedByMetadata())
>       ValueAsMetadata::handleDeletion(this);
>   
>     // Remove associated metadata from context.
>     if (HasMetadata)
>       clearMetadata();
>
> Maybe that commit should be reverted.

Thank you! It looks like so indeed. Basically `ReplaceableMetadataImpl::replaceAllUsesWith` removed the null Value from the metadata nodes, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108642