Re: [PATCH 03/24] RISC-V sim: Atomic fixes.

[Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org>]

On 21 Apr 2021 16:00, Jim Wilson wrote:
> On Sun, Apr 18, 2021 at 8:56 PM Mike Frysinger wrote:
> > On 17 Apr 2021 10:58, Jim Wilson wrote:
> > > Handle aq and rl.  Fix wrong value when rd is zero register.  Fix
> > > amoswap when rd and rs2 are the same register.
> >
> > lgtm.  can you add tests for these things, and for the other fixes ?
> 
> Tests for these amo issues would be useful, as this isn't well tested by
> the gcc testsuite.  I will look into that.
> 
> Most of the other stuff is well tested just by running the gcc testsuite.

imo, "just run the gcc testsuite" is not reasonable.  it is extremely large
and slow and not conducive to iterative/fast development.   it happening to
hit a bug sometimes depending on the current codegen behavior isn't the same
as having good targetted logic.

that isn't to say the gcc testsuite isn't valuable.  it certainly is as a
large stress when you're pretty confident that the sim/whatever is ready to
run against it.  but i'm not running it every day (or even some months) when
working on the sim, and most projects i've worked on run it daily at best on
servers.

that said, i'm not advocating for 1000% coverage on the sim side.  having a
few important high level things to start with is fine, and then taking the
approach like gcc where we fill it out based on regressions.

> I'm working on the gdb sim as a side project while working on other
> projects, and if I have to write a lot of tests there could be a
> significant delay in completing the patch set.  I'd prefer if some of this
> stuff could be handled with follow up patches.

my concern is that this becomes a convenient "we'll get to it later" and then
later never comes.  i'm not saying you don't have good intentions, but they
often don't last with real world pressures.  the only carrot/leverage that
exists is not yet being merged.

so maybe, maybe, you get a pass now, but this isn't the status quo moving
forward.
-mike