Re: [PULL 00/63] riscv-to-apply queue

[Philippe Mathieu-Daudé]

On 8/3/20 7:53 PM, Thomas Huth wrote:
> On 30/06/2020 10.44, LIU Zhiwei wrote:
>>
>>
>> On 2020/6/30 16:11, Thomas Huth wrote:
>>> On 30/06/2020 08.56, LIU Zhiwei wrote:
>>>>
>>>>
>>>> On 2020/6/29 6:51, Alistair Francis wrote:
>>>>> On Sun, Jun 28, 2020 at 7:30 AM Peter Maydell
>>>>> <peter.maydell@linaro.org> wrote:
>>>>>> On Fri, 26 Jun 2020 at 22:53, Alistair Francis
>>>>>> <alistair.francis@wdc.com> wrote:
>>>>>>> The following changes since commit
>>>>>>> 553cf5d7c47bee05a3dec9461c1f8430316d516b:
>>>>>>>
>>>>>>>    Merge remote-tracking branch
>>>>>>> 'remotes/pmaydell/tags/pull-target-arm-20200626' into staging
>>>>>>> (2020-06-26 18:22:36 +0100)
>>>>>>>
>>>>>>> are available in the Git repository at:
>>>>>>>
>>>>>>>    git@github.com:alistair23/qemu.git
>>>>>>> tags/pull-riscv-to-apply-20200626-1
>>>>>>>
>>>>>>> for you to fetch changes up to
>>>>>>> b39d59434ea10649fdb9e0a339c30c76e38c5e17:
>>>>>>>
>>>>>>>    target/riscv: configure and turn on vector extension from
>>>>>>> command line (2020-06-26 14:22:15 -0700)
>>>>>>>
>>>>>>> ----------------------------------------------------------------
>>>>>>> This PR contains two patches to improve PLIC support in QEMU.
>>>>>>>
>>>>>>> The rest of the PR is adding support for the v0.7.1 RISC-V vector
>>>>>>> extensions. This is experimental support as the vector extensions are
>>>>>>> still in a draft state.
>>>>>>>
>>>>>> Hi; I'm afraid this fails to build on PPC64 and s390x (ie
>>>>>> our big-endian hosts):
>>>> Hi Peter,
>>>>
>>>> Do you mean you built the patch set on PPC64 or s390x and got errors
>>>> in the list? Or just a worry?
>>>>
>>>> I have built the patch set on Ubuntu 18.04 X86-64. I don't know which
>>>> compile option
>>>> will fails the compilation. If you compiled on Ubuntu x86-64, could
>>>> you show me the
>>>> compile option?
>>>
>>> The related code in your patch "target/riscv: add vector stride load
>>> and store instructions" is in a "#ifdef HOST_WORDS_BIGENDIAN" section,
>>> so of course this bug does not trigger on a x86 host. You could
>>> temporarily turn the "#ifdef HOST_WORDS_BIGENDIAN" into a "#if 1" to
>>> see whether you can then also reproduce the error on x86.
>>>
>> Yes. I can reproduce it in this way.
>>>> As a note: I try to find a PPC64 for test, but I'm afraid it will be
>>>> too later.
>>>> Is there an available  PPC64  machine in the community?
>>>
>>> Maybe the easiest way to test your code on a big endian machine, too,
>>> is to get a github account, clone the QEMU repository there, and
>>> enable Travis for that repo. Then your code gets built on some non-x86
>>> architectures (including a big-endian s390x) as soon as you push it to
>>> the repo (see .travis.yml for details).
>>>
>> Thanks very much. I will have a try.
>>>>>> /home/ubuntu/qemu/target/riscv/vector_helper.c: In function
>>>>>> ‘vext_clear’:
>>>>>> /home/ubuntu/qemu/target/riscv/vector_helper.c: In function
>>>>>> ‘vext_clear’:
>>>>>> /home/ubuntu/qemu/target/riscv/vector_helper.c:154:21: error: invalid
>>>>>> operands to binary & (have ‘void *’ and ‘long long unsigned int’)
>>>>>>           memset(tail & ~(7ULL), 0, part1);
>>>
>>> You obviously must not use "&" with a pointer. I guess you have to
>>> cast to "uintptr_t" and back, or think of some other smart way to fix
>>> this.
>>>
>> Yes. That's the error. It  build successfully after the cast.
> 
> I'm sorry, but the new code fails to compile on big endian 32-bit
> targets, see https://gitlab.com/huth/qemu/-/jobs/667762754#L3434 :
> 
> /builds/huth/qemu/target/riscv/vector_helper.c: In function 'vext_clear':
> /builds/huth/qemu/target/riscv/vector_helper.c:154:16: error: cast to
> pointer from integer of different size [-Werror=int-to-pointer-cast]
>          memset((void *)((uintptr_t)tail & ~(7ULL)), 0, part1);
>                 ^
> /builds/huth/qemu/target/riscv/vector_helper.c:155:16: error: cast to
> pointer from integer of different size [-Werror=int-to-pointer-cast]
>         memset((void *)(((uintptr_t)tail + 8) & ~(7ULL)), 0, part2);
>                 ^
> cc1: all warnings being treated as errors
> 
> A quick work-around is maybe to replace "ULL" with simply "UL" ?

Or use QEMU_ALIGN_PTR_DOWN?

> 
>  Thomas
> 
>