[PATCH] D67764: [LV] Runtime checks with OptForSize

Sjoerd Meijer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 21 13:28:29 PDT 2019


SjoerdMeijer added a comment.

> Which commit does this trace back to, D65197 <https://reviews.llvm.org/D65197>?

This is the result of D65197 <https://reviews.llvm.org/D65197> and D66803 <https://reviews.llvm.org/D66803>. The former was the functional change, the latter changed the assert that is now triggered.

> Add a reference to the problem @uabelho reported.

There is no corresponding PR for this. The problem reported by @uabelho is added here as a test-case/regression test.

> Why is the current logic placed too late, and should it be removed if moved earlier as suggested?

Initially I didn't understand this remark, because I thought I had moved the new logic to happen earlier. But having thought about it, I think your question actually is why the existing logic doesn't work, why it doesn't capture this case as it looks like it should. Thus, this extra bit of logic added in this patch shouldn't be necessary, at least not in this form.

And I agree with that. I think in the existing logic there's a `F->hasOptSize()` check missing. I am going to look again...


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

https://reviews.llvm.org/D67764





More information about the llvm-commits mailing list