[Qemu-devel] [PATCH] util/cacheinfo: Fix warning generated by clang

[Qemu-devel] [PATCH] util/cacheinfo: Fix warning generated by clang

[Pranith Kumar]

Clang generates the following warning on aarch64 host:

  CC      util/cacheinfo.o
/home/pranith/qemu/util/cacheinfo.c:121:48: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
        asm volatile("mrs\t%0, ctr_el0" : "=r"(ctr));
                                               ^
/home/pranith/qemu/util/cacheinfo.c:121:28: note: use constraint modifier "w"
        asm volatile("mrs\t%0, ctr_el0" : "=r"(ctr));
                           ^~
                           %w0

Constraint modifier 'w' is not (yet?) accepted by gcc. Fix this by increasing the ctr size.

Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
 util/cacheinfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/cacheinfo.c b/util/cacheinfo.c
index f987522df4..6253049533 100644
--- a/util/cacheinfo.c
+++ b/util/cacheinfo.c
@@ -112,7 +112,7 @@ static void sys_cache_info(int *isize, int *dsize)
 static void arch_cache_info(int *isize, int *dsize)
 {
     if (*isize == 0 || *dsize == 0) {
-        unsigned ctr;
+        unsigned long ctr;
 
         /* The real cache geometry is in CCSIDR_EL1/CLIDR_EL1/CSSELR_EL1,
            but (at least under Linux) these are marked protected by the
-- 
2.13.0

Re: [Qemu-devel] [PATCH] util/cacheinfo: Fix warning generated by clang

[Emilio G. Cota]

On Fri, Jun 30, 2017 at 11:39:46 -0400, Pranith Kumar wrote:
> Clang generates the following warning on aarch64 host:
> 
>   CC      util/cacheinfo.o
> /home/pranith/qemu/util/cacheinfo.c:121:48: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>         asm volatile("mrs\t%0, ctr_el0" : "=r"(ctr));
>                                                ^
> /home/pranith/qemu/util/cacheinfo.c:121:28: note: use constraint modifier "w"
>         asm volatile("mrs\t%0, ctr_el0" : "=r"(ctr));
>                            ^~
>                            %w0
> 
> Constraint modifier 'w' is not (yet?) accepted by gcc. Fix this by increasing the ctr size.
> 
> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>

I can reproduce with clang 3.9.1.

Tested-by: Emilio G. Cota <cota@braap.org>
Reviewed-by: Emilio G. Cota <cota@braap.org>

Thanks,

		Emilio

Re: [Qemu-devel] [PATCH] util/cacheinfo: Fix warning generated by clang

[Richard Henderson]

On 06/30/2017 08:39 AM, Pranith Kumar wrote:
> Clang generates the following warning on aarch64 host:
> 
>    CC      util/cacheinfo.o
> /home/pranith/qemu/util/cacheinfo.c:121:48: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
>          asm volatile("mrs\t%0, ctr_el0" : "=r"(ctr));
>                                                 ^
> /home/pranith/qemu/util/cacheinfo.c:121:28: note: use constraint modifier "w"
>          asm volatile("mrs\t%0, ctr_el0" : "=r"(ctr));
>                             ^~
>                             %w0

That is an absolutely stupid warning.  There's long precedent for the compiler 
choosing the prefix for you based on the type of the argument.

> 
> Constraint modifier 'w' is not (yet?) accepted by gcc. Fix this by increasing the ctr size.

Certainly it is -- since the beginning of time.


r~

Re: [Qemu-devel] [PATCH] util/cacheinfo: Fix warning generated by clang

[Peter Maydell]

On 1 July 2017 at 23:20, Richard Henderson <rth@twiddle.net> wrote:
> On 06/30/2017 08:39 AM, Pranith Kumar wrote:
>>
>> Clang generates the following warning on aarch64 host:
>>
>>    CC      util/cacheinfo.o
>> /home/pranith/qemu/util/cacheinfo.c:121:48: warning: value size does not
>> match register size specified by the constraint and modifier
>> [-Wasm-operand-widths]
>>          asm volatile("mrs\t%0, ctr_el0" : "=r"(ctr));
>>                                                 ^
>> /home/pranith/qemu/util/cacheinfo.c:121:28: note: use constraint modifier
>> "w"
>>          asm volatile("mrs\t%0, ctr_el0" : "=r"(ctr));
>>                             ^~
>>                             %w0
>
>
> That is an absolutely stupid warning.  There's long precedent for the
> compiler choosing the prefix for you based on the type of the argument.

Isn't that the problem? The type of the argument says "32 bits"
but the instruction here really wants 64 bits (MRS takes Xn, not Wn).

thanks
-- PMM

Re: [Qemu-devel] [PATCH] util/cacheinfo: Fix warning generated by clang

[Richard Henderson]

On 07/01/2017 03:30 PM, Peter Maydell wrote:
> On 1 July 2017 at 23:20, Richard Henderson <rth@twiddle.net> wrote:
>> On 06/30/2017 08:39 AM, Pranith Kumar wrote:
>>>
>>> Clang generates the following warning on aarch64 host:
>>>
>>>     CC      util/cacheinfo.o
>>> /home/pranith/qemu/util/cacheinfo.c:121:48: warning: value size does not
>>> match register size specified by the constraint and modifier
>>> [-Wasm-operand-widths]
>>>           asm volatile("mrs\t%0, ctr_el0" : "=r"(ctr));
>>>                                                  ^
>>> /home/pranith/qemu/util/cacheinfo.c:121:28: note: use constraint modifier
>>> "w"
>>>           asm volatile("mrs\t%0, ctr_el0" : "=r"(ctr));
>>>                              ^~
>>>                              %w0
>>
>>
>> That is an absolutely stupid warning.  There's long precedent for the
>> compiler choosing the prefix for you based on the type of the argument.
> 
> Isn't that the problem? The type of the argument says "32 bits"
> but the instruction here really wants 64 bits (MRS takes Xn, not Wn).

The warning is telling me to use %w to force Wn.  So if the assembler really 
doesn't like Wn, the warning is a bit more than confusing.

Perhaps it ought to be telling me to use %x to force Xn in spite of the type?


r~

Re: [Qemu-devel] [PATCH] util/cacheinfo: Fix warning generated by clang

[Peter Maydell]

On 1 July 2017 at 23:35, Richard Henderson <rth@twiddle.net> wrote:
> On 07/01/2017 03:30 PM, Peter Maydell wrote:
>>
>> On 1 July 2017 at 23:20, Richard Henderson <rth@twiddle.net> wrote:
>>> That is an absolutely stupid warning.  There's long precedent for the
>>> compiler choosing the prefix for you based on the type of the argument.
>>
>>
>> Isn't that the problem? The type of the argument says "32 bits"
>> but the instruction here really wants 64 bits (MRS takes Xn, not Wn).
>
>
> The warning is telling me to use %w to force Wn.  So if the assembler really
> doesn't like Wn, the warning is a bit more than confusing.

Wouldn't be the first time a compiler has produced a confusing warning :-)

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63359 includes some
previous gcc-vs-clang-dev discussion on the topic of the warning.
It looks like the clang dev rationale is that having %0 always
generate a 64-bit register access even when passed a 32-bit value
is confusing (eg people expect "str %0, [addr]" : ... : "r" (var_32bits)"
to do a 32 bit store, not a 64 bit store), so better to warn and
nudge the code author into being explicit about the size they wanted.

> Perhaps it ought to be telling me to use %x to force Xn in spite of the
> type?

You always get Xn anyway, regardless of the type.

For us, I think the right thing to do is make 'ctr' be a uint64_t,
because we're reading a 64 bit sysreg and silently truncating it
as a side effect of the asm constraints is a bit obscure.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] util/cacheinfo: Fix warning generated by clang

[Richard Henderson]

On 07/01/2017 03:44 PM, Peter Maydell wrote:
> On 1 July 2017 at 23:35, Richard Henderson <rth@twiddle.net> wrote:
>> Perhaps it ought to be telling me to use %x to force Xn in spite of the
>> type?
> 
> You always get Xn anyway, regardless of the type.
> 
> For us, I think the right thing to do is make 'ctr' be a uint64_t,
> because we're reading a 64 bit sysreg and silently truncating it
> as a side effect of the asm constraints is a bit obscure.

Fair enough.  Applied as-is to tcg-next.


r~