Messages in this thread | | | Date | Sat, 24 Jan 2009 18:27:58 +0100 | From | Ingo Molnar <> | Subject | Re: 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/
| |