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

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 16 15:03:54 PST 2020


ABataev added a comment.

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

> In D69585#1825133 <https://reviews.llvm.org/D69585#1825133>, @ABataev wrote:
>
> > 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.
>
>
> It's not included in the latest version of the patch. As written above, I'm reasonably sure I was mistaken about the need for a flag, and it should be ok to simply do the change unconditionally. I can put the flag back just for the purpose of the tests if you want, that'd certainly make handling of the tests trivial, but then the tests wouldn't really test "normal" PCHs, so do you really want that?


Of course not. I was just wandering if you still going to use a flag. If you're not going to use it, then there is only one choice - update the tests. The tests are written so to test that emitting/including PCH does not break the codegen. So it should be tested.


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

https://reviews.llvm.org/D69585





More information about the cfe-commits mailing list