diff mbox series

[v2,07/13] target/riscv: Include missing 'accel/tcg/getpc.h' in csr.c

Message ID 20250424094653.35932-8-philmd@linaro.org
State Superseded
Headers show
Series include: Remove "exec/exec-all.h" | expand

Commit Message

Philippe Mathieu-Daudé April 24, 2025, 9:46 a.m. UTC 
"accel/tcg/getpc.h" is pulled in indirectly. Include it
explicitly to avoid when refactoring unrelated headers:

  target/riscv/csr.c:2117:25: error: call to undeclared function 'GETPC' [-Wimplicit-function-declaration]
   2117 |     if ((val & RVC) && (GETPC() & ~3) != 0) {
        |                         ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/riscv/csr.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Philippe Mathieu-Daudé April 24, 2025, 9:50 a.m. UTC | #1 
+qemu-riscv list

On 24/4/25 11:46, Philippe Mathieu-Daudé wrote:
> "accel/tcg/getpc.h" is pulled in indirectly. Include it
> explicitly to avoid when refactoring unrelated headers:
> 
>    target/riscv/csr.c:2117:25: error: call to undeclared function 'GETPC' [-Wimplicit-function-declaration]
>     2117 |     if ((val & RVC) && (GETPC() & ~3) != 0) {
>          |                         ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/riscv/csr.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index c52c87faaea..13086438552 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -28,6 +28,7 @@
>   #include "exec/cputlb.h"
>   #include "exec/tb-flush.h"
>   #include "exec/icount.h"
> +#include "accel/tcg/getpc.h"
>   #include "qemu/guest-random.h"
>   #include "qapi/error.h"
>   #include <stdbool.h>
Mark Cave-Ayland April 24, 2025, 10:14 a.m. UTC | #2 
On 24/04/2025 10:46, Philippe Mathieu-Daudé wrote:

> "accel/tcg/getpc.h" is pulled in indirectly. Include it
> explicitly to avoid when refactoring unrelated headers:
> 
>    target/riscv/csr.c:2117:25: error: call to undeclared function 'GETPC' [-Wimplicit-function-declaration]
>     2117 |     if ((val & RVC) && (GETPC() & ~3) != 0) {
>          |                         ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/riscv/csr.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index c52c87faaea..13086438552 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -28,6 +28,7 @@
>   #include "exec/cputlb.h"
>   #include "exec/tb-flush.h"
>   #include "exec/icount.h"
> +#include "accel/tcg/getpc.h"
>   #include "qemu/guest-random.h"
>   #include "qapi/error.h"
>   #include <stdbool.h>

I'm mildly curious as to why the target needs to include 
accel/tcg/getpc.h directly as it's almost a requirement for TCG, but sure:

Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>


ATB,

Mark.
Alistair Francis April 24, 2025, 10:23 a.m. UTC | #3 
On Thu, Apr 24, 2025 at 7:48 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> "accel/tcg/getpc.h" is pulled in indirectly. Include it
> explicitly to avoid when refactoring unrelated headers:
>
>   target/riscv/csr.c:2117:25: error: call to undeclared function 'GETPC' [-Wimplicit-function-declaration]
>    2117 |     if ((val & RVC) && (GETPC() & ~3) != 0) {
>         |                         ^
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/csr.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index c52c87faaea..13086438552 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -28,6 +28,7 @@
>  #include "exec/cputlb.h"
>  #include "exec/tb-flush.h"
>  #include "exec/icount.h"
> +#include "accel/tcg/getpc.h"
>  #include "qemu/guest-random.h"
>  #include "qapi/error.h"
>  #include <stdbool.h>
> --
> 2.47.1
>
>
Philippe Mathieu-Daudé April 24, 2025, 8:13 p.m. UTC | #4 
On 24/4/25 12:14, Mark Cave-Ayland wrote:
> On 24/04/2025 10:46, Philippe Mathieu-Daudé wrote:
> 
>> "accel/tcg/getpc.h" is pulled in indirectly. Include it
>> explicitly to avoid when refactoring unrelated headers:
>>
>>    target/riscv/csr.c:2117:25: error: call to undeclared function 
>> 'GETPC' [-Wimplicit-function-declaration]
>>     2117 |     if ((val & RVC) && (GETPC() & ~3) != 0) {
>>          |                         ^
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/riscv/csr.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> index c52c87faaea..13086438552 100644
>> --- a/target/riscv/csr.c
>> +++ b/target/riscv/csr.c
>> @@ -28,6 +28,7 @@
>>   #include "exec/cputlb.h"
>>   #include "exec/tb-flush.h"
>>   #include "exec/icount.h"
>> +#include "accel/tcg/getpc.h"
>>   #include "qemu/guest-random.h"
>>   #include "qapi/error.h"
>>   #include <stdbool.h>
> 
> I'm mildly curious as to why the target needs to include accel/tcg/ 
> getpc.h directly as it's almost a requirement for TCG

Indeed. There is a TODO around, added upon introduction in
commit f18637cd611 ("RISC-V: Add misa runtime write support"):

2113     /*
2114      * Suppress 'C' if next instruction is not aligned
2115      * TODO: this should check next_pc
2116      */
2117     if ((val & RVC) && (GETPC() & ~3) != 0) {
2118         val &= ~RVC;
2119     }


> Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>

Thanks!
Richard Henderson April 24, 2025, 9:01 p.m. UTC | #5 
On 4/24/25 13:13, Philippe Mathieu-Daudé wrote:
> On 24/4/25 12:14, Mark Cave-Ayland wrote:
>> On 24/04/2025 10:46, Philippe Mathieu-Daudé wrote:
>>
>>> "accel/tcg/getpc.h" is pulled in indirectly. Include it
>>> explicitly to avoid when refactoring unrelated headers:
>>>
>>>    target/riscv/csr.c:2117:25: error: call to undeclared function 'GETPC' [-Wimplicit- 
>>> function-declaration]
>>>     2117 |     if ((val & RVC) && (GETPC() & ~3) != 0) {
>>>          |                         ^
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   target/riscv/csr.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>>> index c52c87faaea..13086438552 100644
>>> --- a/target/riscv/csr.c
>>> +++ b/target/riscv/csr.c
>>> @@ -28,6 +28,7 @@
>>>   #include "exec/cputlb.h"
>>>   #include "exec/tb-flush.h"
>>>   #include "exec/icount.h"
>>> +#include "accel/tcg/getpc.h"
>>>   #include "qemu/guest-random.h"
>>>   #include "qapi/error.h"
>>>   #include <stdbool.h>
>>
>> I'm mildly curious as to why the target needs to include accel/tcg/ getpc.h directly as 
>> it's almost a requirement for TCG
> 
> Indeed. There is a TODO around, added upon introduction in
> commit f18637cd611 ("RISC-V: Add misa runtime write support"):
> 
> 2113     /*
> 2114      * Suppress 'C' if next instruction is not aligned
> 2115      * TODO: this should check next_pc
> 2116      */
> 2117     if ((val & RVC) && (GETPC() & ~3) != 0) {
> 2118         val &= ~RVC;
> 2119     }

Yes, I've sent mail about this bug at least twice.

r~
diff mbox series

Patch

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index c52c87faaea..13086438552 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -28,6 +28,7 @@ 
 #include "exec/cputlb.h"
 #include "exec/tb-flush.h"
 #include "exec/icount.h"
+#include "accel/tcg/getpc.h"
 #include "qemu/guest-random.h"
 #include "qapi/error.h"
 #include <stdbool.h>