[PATCH] D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm

Mon Jun 15 19:51:10 PDT 2020

greened added a comment.

In D68230#2081496 <https://reviews.llvm.org/D68230#2081496>, @lebedev.ri wrote:

>

> Like this was already disscussed (i think?) in some -dev thread, how would that work?

I must have missed that message.

> If such tests are in test-suite, why would they need `update_cc_test_checks.py`?

For the same reason any other tests needs it.  The compiler changes and sometimes we need to update tests.

> If they need `update_cc_test_checks.py`, then now everyone also needs to run those tests before commit?

Not at all.  The discussion seemed to settle on the bots running test-suite to be sufficient.  If one of them fails, then yes, the author would have to address the test, just like any other test in test-suite.

> And what happens if they break? Just silently regenerate them?

What do we do for such tests in clang/LLVM today?  It's exactly the same situation.  I am of the opinion that we do far too much blind regeneration of tests and I have a bunch of follow-on commits that will hopefully improve that situation by making update_*_tests.py more featureful and able to produce smaller, more robust tests.

> Also, test-suite is not in monorepo, so you can't update those tests at the same time as making llvm change.

Yes, that's a consequence of putting them in test-suite.  My personal preference would be to put them in the monorepo but the community seemed to decide that's not the right place, at least for now.

> TLDR: i don't think the burden of proof has been provided here.

I'm not sure what "proof" you're looking for.  There seems to be fairly wide agreement that asm-style tests are useful.  The controversey, if any, is about where they are placed.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68230