| Message ID | 20170630153946.11997-1-bobby.prani@gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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~
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
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~
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
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~
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
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(-)