[llvm] [ThinLTO] optimize propagateAttributes performance (PR #132917)

Thu Mar 27 08:19:21 PDT 2025

teresajohnson wrote:

On Thu, Mar 27, 2025 at 4:38 AM Zhaoxuan Jiang ***@***.***>
wrote:

> I managed to grab some interesting profiling data with a RelWithDebInfo
> libLTO.dylib.
> Baseline
>
> image.png (view on web)
> <https://github.com/user-attachments/assets/def0f19c-b6b0-43de-879a-b3febde4ded0>
>
> Most time are spent in insert:
>
> image.png (view on web)
> <https://github.com/user-attachments/assets/f3063d33-6ef7-4fe2-8cd8-98355fc741bb>
>
> 19.52 min   68.9%        0 s                             llvm::detail::DenseSetImpl<llvm::ValueInfo, llvm::DenseMap<llvm::ValueInfo, llvm::detail::DenseSetEmpty, llvm::DenseMapInfo<llvm::ValueInfo, void>, llvm::detail::DenseSetPair<llvm::ValueInfo>>, llvm::DenseMapInfo<llvm::ValueInfo, void>>::insert(llvm::ValueInfo const&)
> 1.15 min    4.0%        0 s                             llvm::detail::DenseSetImpl<llvm::ValueInfo, llvm::DenseMap<llvm::ValueInfo, llvm::detail::DenseSetEmpty, llvm::DenseMapInfo<llvm::ValueInfo, void>, llvm::detail::DenseSetPair<llvm::ValueInfo>>, llvm::DenseMapInfo<llvm::ValueInfo, void>>::contains(llvm::ValueInfo const&) const
>
> MarkedNonReadWriteOnly in GlobalValueSummaryInfo
>
> image.png (view on web)
> <https://github.com/user-attachments/assets/06f5af9d-a73d-478c-9288-e34bb74d9400>
> MarkedNonReadWriteOnly in ValueInfo
>
> image.png (view on web)
> <https://github.com/user-attachments/assets/4c20c3e2-7c65-44cd-9b49-53fd80e875d1>
>
> image.png (view on web)
> <https://github.com/user-attachments/assets/c5a4ab1d-8fbf-4cb7-90ab-aec763c34188>
>
> I strongly suspect that the relationship between ValueInfo and
> GlobalValueSummaryInfo is not strictly 1-to-1, otherwise the 10x
> performance difference cannot be explained. This is also the reason why I
> initially decided to declare the flag in GlobalValueSummaryInfo:
> DenseMapInfo<ValueInfo> utilizes the pointer identity of
> GlobalValueSummaryMapTy entry, so the MarkedNonReadWriteOnly set was
> effectively marking the underlying GlobalValueSummaryInfo rather than
> ValueInfo. However, I am uncertain whether this is the expected behavior.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/llvm/llvm-project/pull/132917#issuecomment-2757731781>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AE37ZQ5376UH7TNMBDDCGWT2WPPL5AVCNFSM6AAAAABZXJM6NGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJXG4ZTCNZYGE>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
> [image: nocchijiang]*nocchijiang* left a comment
> (llvm/llvm-project#132917)
> <https://github.com/llvm/llvm-project/pull/132917#issuecomment-2757731781>
>
> I managed to grab some interesting profiling data with a RelWithDebInfo
> libLTO.dylib.
> Baseline
>
> image.png (view on web)
> <https://github.com/user-attachments/assets/def0f19c-b6b0-43de-879a-b3febde4ded0>
>
> Most time are spent in insert:
>
> image.png (view on web)
> <https://github.com/user-attachments/assets/f3063d33-6ef7-4fe2-8cd8-98355fc741bb>
>
> 19.52 min   68.9%        0 s                             llvm::detail::DenseSetImpl<llvm::ValueInfo, llvm::DenseMap<llvm::ValueInfo, llvm::detail::DenseSetEmpty, llvm::DenseMapInfo<llvm::ValueInfo, void>, llvm::detail::DenseSetPair<llvm::ValueInfo>>, llvm::DenseMapInfo<llvm::ValueInfo, void>>::insert(llvm::ValueInfo const&)
> 1.15 min    4.0%        0 s                             llvm::detail::DenseSetImpl<llvm::ValueInfo, llvm::DenseMap<llvm::ValueInfo, llvm::detail::DenseSetEmpty, llvm::DenseMapInfo<llvm::ValueInfo, void>, llvm::detail::DenseSetPair<llvm::ValueInfo>>, llvm::DenseMapInfo<llvm::ValueInfo, void>>::contains(llvm::ValueInfo const&) const
>
> MarkedNonReadWriteOnly in GlobalValueSummaryInfo
>
> image.png (view on web)
> <https://github.com/user-attachments/assets/06f5af9d-a73d-478c-9288-e34bb74d9400>
> MarkedNonReadWriteOnly in ValueInfo
>
> image.png (view on web)
> <https://github.com/user-attachments/assets/4c20c3e2-7c65-44cd-9b49-53fd80e875d1>
>
> image.png (view on web)
> <https://github.com/user-attachments/assets/c5a4ab1d-8fbf-4cb7-90ab-aec763c34188>
>
> I strongly suspect that the relationship between ValueInfo and
> GlobalValueSummaryInfo is not strictly 1-to-1, otherwise the 10x
> performance difference cannot be explained. This is also the reason why I
> initially decided to declare the flag in GlobalValueSummaryInfo:
> DenseMapInfo<ValueInfo> utilizes the pointer identity of
> GlobalValueSummaryMapTy entry, so the MarkedNonReadWriteOnly set was
> effectively marking the underlying GlobalValueSummaryInfo rather than
> ValueInfo. However, I am uncertain whether this is the expected behavior.
>

Sorry for the noise on this suggestion - thinking it through more it
doesn't make a lot of sense to put in the ValueInfo. The
GlobalValueSummaryInfo structs are the values in the index map, and each
ValueInfo is essentially a pointer to one of them, along with a few bits,
and those bits can vary by the characteristics of the reference. They are
also meant to be immutable after building the summary. As you note the
ValueInfo comparisons only look at the pointer value, not these bits, which
is why a set of them works to unique the entries to the underlying summary
index map entries.

I need to think about this some more, as stashing temporary analysis
results in these structures isn't ideal. I'm still trying to understand how
this code could be so slow, compared to everything else we do in the
indexing step. Since there can be at most one entry in the
MarkedNonReadWriteOnly set per summary index map (`GlobalValueSummaryMapTy
GlobalValueMap`) entry, the actual inserting in theory shouldn't be
massively slower than building that map in the first place. So it must just
be an extreme number of lookups during insert attempts that is causing it
to be so slow? Can you collect some stats: how big the set is at the end,
how many times propagateAttributesToRefs is called, how many total refs()
get processed in that loop, and how many of those have
`!VI.getAccessSpecifier()`?

—
> Reply to this email directly, view it on GitHub
> <https://github.com/llvm/llvm-project/pull/132917#issuecomment-2757731781>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AE37ZQ5376UH7TNMBDDCGWT2WPPL5AVCNFSM6AAAAABZXJM6NGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJXG4ZTCNZYGE>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>

-- 
Teresa Johnson |  Software Engineer |  ***@***.*** |

https://github.com/llvm/llvm-project/pull/132917