On Wed, 29 Aug 2018 09:52:00 PDT (-0700), Jim Wilson wrote:
On Wed, Aug 29, 2018 at 9:22 AM, Palmer Dabbelt <pal...@sifive.com> wrote:
Thanks Jim -- I'm afraid at least part of this was my mess, as I had to go
add in the ZERO_EXTEND_LOAD hackery to work around some bug in this file
that I couldn't figure out how to fix in a better way.

ZERO_EXTEND_LOAD is exactly the same as SUBX, so is redundant.  That
is the main reason why I dropped it.

Ah, OK, this makes a bit more sense then.

IIRC the issue I found was in booting a Linux kernel that was built with
"-mcmodel=medany".  If I'm reading our current kernel port correctly that's
the default for 64-bit systems, but I'm not sure when we flipped over so I
think it's worth checking -- I know at the time I fixed the bug it wasn't
the default :).  You should be able to do a "make V=1" inside Linux to just
see the compiler command lines.

By default, 32-bit kernels are built medlow, and 64-bit kernels are
built medany.  That is in the riscv Kconfig file.  Before committing
the patch, I did apply it to freedom-u-sdk, build a 64-bit kernel, and
boot both on qemu and spike, so I think we are OK here.

Yep, as long as that's the case for whatever kernel you built then we're OK.

I did find a few optimization problems via the gcc testsuite while
trying to improve the file.  The most interesting one, and maybe the
one you hit earlier, is with non-extended loads, which because of
LOAD_EXTEND_OP we actually have to use zero-extending loads for
char/short loads, but sign-extending loads for words, and I had to add
a new mode attribute for that.  That one took a little time to debug
because it didn't fail with trivial testcases, and the miscompilation
was non-obvious.

That sounds very much like what I ran in to.

Thanks!

Reply via email to