[clang] [Serialization] Load Specializations Lazily (PR #76774)
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Sun Feb 18 00:56:54 PST 2024
ChuanqiXu9 wrote:
> > > > > > > > > [do not merge] [runtime-cxxmodules] Rework our lazy template specialization deserialization mechanism [root-project/root#14495](https://github.com/root-project/root/pull/14495)
> > > > > > > >
> > > > > > > >
> > > > > > > > From [root-project/root#14495](https://github.com/root-project/root/pull/14495), I see there is new reply saying the testing is actually fine. Do you think we still need to split the patch?
> > > > > > >
> > > > > > >
> > > > > > > That comment was concerning the version of the patch that had the lazy template deserialization turned off by default. Yes, I still think that this patch should implement tha on-disk hash table on top of D41416
> > > > > >
> > > > > >
> > > > > > OK. And would you like to send a PR for D41416? I've already fixed the issue mentioned in the review page. Then I'd like to send small and incremental patches on that.
> > > > >
> > > > >
> > > > > Do you mean that I should open a PR for D41416 and you will apply your patch there? I have no problem if we do everything here as part of this PR. This way we will have the full history of how this was born in one place ;)
> > > >
> > > >
> > > > Yeah, and please create a branch under llvm/llvm-project directly. Then I can perform stacked PR on that.
> > >
> > >
> > > There it is: https://github.com/llvm/llvm-project/tree/users/vgvassilev/D41416_D153003
> >
> >
> > Is there a PR? So that we can comment on that. For example, is D153003 necessary? I remeber in the review process, we think it may not be wanted.
>
> I can create a PR. My understanding is that D153003 because of the intricacies of the ODR hashing approach. If we create an alternative we can take it out. For now, we can keep it and later we can drop it.
I feel better to drop D153003 if it is not a blocking issue. I feel it make things more complicated...
https://github.com/llvm/llvm-project/pull/76774
More information about the cfe-commits
mailing list