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

[Richard Henderson <richard.henderson@linaro.org>]

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~