[PATCH] D69585: PerformPendingInstatiations() already in the PCH

Thu Jan 16 14:15:30 PST 2020

ABataev added a comment.

In D69585#1823969 <https://reviews.llvm.org/D69585#1823969>, @llunak wrote:

> In D69585#1821831 <https://reviews.llvm.org/D69585#1821831>, @aganea wrote:
>
> > What is the error?
>
>
> I take that part back, actually. I don't quite remember anymore what exactly I did in October, but if I now revert the PCH tweaks in LibreOffice I did to avoid the error, the compilation fails even without the patch or with GCC. So I assume what really happened was that code changes triggered the error and I incorrectly assumed it was because of my patch. So, unless proven otherwise, I take it that my patch is actually technically correct without causing any code regressions (the OpenMP problem has just been fixed).
>
> What remains is those 22 tests which fail because moving the instantiations reorders the code that is expected by FileCheck. This patch handles 2 of them, and it's already rather tedious (the OpenMP tests are large). Is this really the way to handle them, or does somebody have a better idea?

I thought you were going to add an option or a flag to control the behavior? If so, just provide an option in tests to avoid triggering of the new behavior (except for declare_target... test and those 2 you modified already) and that's it.

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

https://reviews.llvm.org/D69585