[PATCH] D95304: [update_llc_test_checks][RISCV] Add new tests

Tue Jan 26 07:25:14 PST 2021

luismarques added a comment.

In D95304#2522347 <https://reviews.llvm.org/D95304#2522347>, @jrtc27 wrote:

> That's... not a good way to fix a bug? This is news to me, and that was committed without any review (let alone by RISC-V maintainers...). And no mention of what the bug actually is in the commit message.

Indeed.

In D95304#2522342 <https://reviews.llvm.org/D95304#2522342>, @jrtc27 wrote:

> Hm, there is a lot of duplication in all these tests (we currently have 12 copies of almost-identical IR), it seems like we should just add a whole load of CHECK lines to a single copy of the IR?..

That would be nice if possible, but IMO that should be a separate patch. Wouldn't that require the bots to support all of the targets to run the combined test? I also wonder if that could become awkward in the future if the outlining started to diverge significantly between targets.

In D95304#2522341 <https://reviews.llvm.org/D95304#2522341>, @arichardson wrote:

> This used to exist but it exposed a bug and the test was removed in b8779337841bea03bc514d239ce76eceba3f01dd <https://reviews.llvm.org/rGb8779337841bea03bc514d239ce76eceba3f01dd>.  The commit message states that it only happened with expensive-checks. If it works (with expensive checks) b8779337841bea03bc514d239ce76eceba3f01dd <https://reviews.llvm.org/rGb8779337841bea03bc514d239ce76eceba3f01dd> can be reverted.

If the builbots with expensive checks are all OK for this patch, and this patch is exactly the same as what b8779337841b <https://reviews.llvm.org/rGb8779337841bea03bc514d239ce76eceba3f01dd> removed, should we then just revert b8779337841b <https://reviews.llvm.org/rGb8779337841bea03bc514d239ce76eceba3f01dd> now?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95304