lkml.org 
[lkml]   [2009]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: inline asm semantics: output constraint width smaller than input

* Török Edwin <edwintorok@gmail.com> wrote:

> On 2009-01-23 20:27, Török Edwin wrote:
> >>>
> >>>
> >> i'd not mind it at all if the kernel could be built with other open-source
> >> compilers too.
> >>
> >> Now in this case the patch you suggest might end up hurting the end result
> >> so it's not an unconditional 'yes'. But ... how much it actually matters
> >> depends on the circumstances.
> >>
> >> So could you please send a sample patch for some of most common inline
> >> assembly statements that are affected by this, so that we can see:
> >>
> >> 1) how ugly the LLVM workarounds are
> >>
> >>
> >
> > Ok, I will prepare a patch for both cases.
> >
> >
> >> 2) how they affect the generated kernel image in practice
> >>
> >> My gut feeling is that it's going to be acceptable with a bit of thinking
> >> (we might even do some wrappers to do this cleanly) - but i'd really like
> >> to see it before giving you that judgement.
> >>
>
> The below patch is to build the kernel for x86_64, with the attached
> .config,
> using llvm-gcc (trunk, with patch from
> http://llvm.org/bugs/show_bug.cgi?id=2989#c2).
>
> The .config has KVM turned off, because I didn't know how to change
> x86_emulate.c so that LLVM builds it
> (http://llvm.org/bugs/show_bug.cgi?id=3373#c10)
> For 32-bit some more changes are required.
>
> The resulting kernel image are of the same size
> $ ls -l vmlinux.patched
> -rwxr-xr-x 1 edwin edwin 11277206 2009-01-24 17:58 vmlinux.patched
> $ ls -l vmlinux
> -rwxr-xr-x 1 edwin edwin 11277206 2009-01-24 18:01 vmlinux
>
> They aren't identical though, a disassembly shows that the address of
> most of functions changed,
> also some register assignments changed (r14 instead of r15, and so on).
>
> Are these changes correct, and are they acceptable?
>
> Best regards,
> --Edwin
>
> ---
> arch/x86/include/asm/uaccess.h | 10 ++++++----
> arch/x86/lib/delay.c | 2 +-
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index 69d2757..28280de 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -154,7 +154,7 @@ extern int __get_user_bad(void);
>
> #define get_user(x, ptr) \
> ({ \
> - int __ret_gu; \
> + unsigned long __ret_gu; \
> unsigned long __val_gu; \
> __chk_user_ptr(ptr); \
> might_fault(); \
> @@ -176,7 +176,7 @@ extern int __get_user_bad(void);
> break; \
> } \
> (x) = (__typeof__(*(ptr)))__val_gu; \
> - __ret_gu; \
> + (int)__ret_gu; \
> })
>
> #define __put_user_x(size, x, ptr, __ret_pu) \
> @@ -239,11 +239,13 @@ extern void __put_user_8(void);
> */
> #define put_user(x, ptr) \
> ({ \
> - int __ret_pu; \
> + __typeof__(*(ptr)) __ret_pu; \

This does not look right. We can sometimes have put_user() of non-integer
types (say structures). How does the (int)__ret_pu cast work in that case?
We'll fall into this branch in that case:

default: \
__put_user_x(X, __pu_val, ptr, __ret_pu); \
break; \

and __ret_pu has a nonsensical type in that case.

> __typeof__(*(ptr)) __pu_val; \
> __chk_user_ptr(ptr); \
> might_fault(); \
> __pu_val = x; \
> + /* return value is 0 or -EFAULT, both fit in 1 byte, and \
> + * are sign-extendable to int */ \
> switch (sizeof(*(ptr))) { \
> case 1: \
> __put_user_x(1, __pu_val, ptr, __ret_pu); \
> @@ -261,7 +263,7 @@ extern void __put_user_8(void);
> __put_user_x(X, __pu_val, ptr, __ret_pu); \
> break; \
> } \
> - __ret_pu; \
> + (int)__ret_pu; \
> })
>
> #define __put_user_size(x, ptr, size, retval, errret) \
> diff --git a/arch/x86/lib/delay.c b/arch/x86/lib/delay.c
> index f456860..12d27f8 100644
> --- a/arch/x86/lib/delay.c
> +++ b/arch/x86/lib/delay.c
> @@ -112,7 +112,7 @@ EXPORT_SYMBOL(__delay);
>
> inline void __const_udelay(unsigned long xloops)
> {
> - int d0;
> + unsigned long d0;
>
> xloops *= 4;
> asm("mull %%edx"

Is this all that you need (plus the 16-bit setup code tweaks) to get LLVM
to successfully build a 64-bit kernel image?

If yes then this doesnt look all that bad or invasive at first sight (if
the put_user() workaround can be expressed in a cleaner way), but in any
case it would be nice to hear an LLVM person's opinion about roughly when
this is going to be solved in LLVM itself.

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-01-24 18:31    [from the cache]
©2003-2011 Jasper Spaans