| Message ID | 1395766541-23979-3-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Deferred, archived |
| Headers | show |
>>> On 25.03.14 at 17:55, <julien.grall@linaro.org> wrote: > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -74,6 +74,7 @@ AFLAGS-y += -D__ASSEMBLY__ -include > $(BASEDIR)/include/xen/config > > # Clang's built-in assembler can't handle .code16/.code32/.code64 yet > AFLAGS-$(clang) += -no-integrated-as > +CFLAGS-$(clang) += -no-integrated-as Iirc Tim had found and worked around other built-in assembler issues in the past, so if this is to be done unconditionally I wonder whether we shouldn't then drop those workarounds. Jan
At 11:53 +0000 on 26 Mar (1395831232), Jan Beulich wrote: > >>> On 25.03.14 at 17:55, <julien.grall@linaro.org> wrote: > > --- a/xen/Rules.mk > > +++ b/xen/Rules.mk > > @@ -74,6 +74,7 @@ AFLAGS-y += -D__ASSEMBLY__ -include > > $(BASEDIR)/include/xen/config > > > > # Clang's built-in assembler can't handle .code16/.code32/.code64 yet > > AFLAGS-$(clang) += -no-integrated-as > > +CFLAGS-$(clang) += -no-integrated-as > > Iirc Tim had found and worked around other built-in assembler issues > in the past, so if this is to be done unconditionally I wonder whether > we shouldn't then drop those workarounds. I would prefer, wherever possible, to make things work with the clang assembler rather than rely on the binutils one forever. BTW, I haven't looked at any of this series in detail yet but I'm planning to go through it all tomorrow. Cheers, Tim.
Hi Tim, On 03/26/2014 01:16 PM, Tim Deegan wrote: > At 11:53 +0000 on 26 Mar (1395831232), Jan Beulich wrote: >>>>> On 25.03.14 at 17:55, <julien.grall@linaro.org> wrote: >>> --- a/xen/Rules.mk >>> +++ b/xen/Rules.mk >>> @@ -74,6 +74,7 @@ AFLAGS-y += -D__ASSEMBLY__ -include >>> $(BASEDIR)/include/xen/config >>> >>> # Clang's built-in assembler can't handle .code16/.code32/.code64 yet >>> AFLAGS-$(clang) += -no-integrated-as >>> +CFLAGS-$(clang) += -no-integrated-as >> >> Iirc Tim had found and worked around other built-in assembler issues >> in the past, so if this is to be done unconditionally I wonder whether >> we shouldn't then drop those workarounds. > > I would prefer, wherever possible, to make things work with the clang > assembler rather than rely on the binutils one forever. The clang integrated assembler is too powerful for some part of Xen :). Every inline assembly code is parsing by the assembler to check the syntax. This will result to failure to generate asm-offsets.c because of the -> in the code (see arch/arm/arm32/asm-offsets.c: DEFINE/BLANK macros). Indeed, the -> is not a valid assembler syntax. > BTW, I haven't looked at any of this series in detail yet but I'm > planning to go through it all tomorrow. There still have few issues to build the tools and Xen x86_64 with clang 3.5. I also would like to see if we can re-enable some warnings (see Config.mk) with newer version of clang. Regards,
>>> On 26.03.14 at 16:08, <julien.grall@linaro.org> wrote: > Hi Tim, > > On 03/26/2014 01:16 PM, Tim Deegan wrote: >> At 11:53 +0000 on 26 Mar (1395831232), Jan Beulich wrote: >>>>>> On 25.03.14 at 17:55, <julien.grall@linaro.org> wrote: >>>> --- a/xen/Rules.mk >>>> +++ b/xen/Rules.mk >>>> @@ -74,6 +74,7 @@ AFLAGS-y += -D__ASSEMBLY__ -include >>>> $(BASEDIR)/include/xen/config >>>> >>>> # Clang's built-in assembler can't handle .code16/.code32/.code64 yet >>>> AFLAGS-$(clang) += -no-integrated-as >>>> +CFLAGS-$(clang) += -no-integrated-as >>> >>> Iirc Tim had found and worked around other built-in assembler issues >>> in the past, so if this is to be done unconditionally I wonder whether >>> we shouldn't then drop those workarounds. >> >> I would prefer, wherever possible, to make things work with the clang >> assembler rather than rely on the binutils one forever. > > The clang integrated assembler is too powerful for some part of Xen :). > Every inline assembly code is parsing by the assembler to check the syntax. > > This will result to failure to generate asm-offsets.c because of the -> > in the code (see arch/arm/arm32/asm-offsets.c: DEFINE/BLANK macros). > Indeed, the -> is not a valid assembler syntax. But what business does the compiler have to pass the assembly code to its internal assembler when all it was asked was to output assembly? That may be acceptable as an optional internal consistency check (verifying the compiler produced valid assembly), but shouldn't constrain the user from using the compiler for things where it's known the output isn't going to be valid assembly. That said, I think it wouldn't be too difficult to change the asm-offsets logic to produce something that does look like valid assembly. Jan
>>> On 27.03.14 at 19:01, <tim@xen.org> wrote: > The patch below works for me (at least as far as building > asm-offsets.h on x86) by wrapping everything in a string. I did try > just prefixing with '#' but clang 3.5 also strips the comments out. > That seems unhelpful, since I know some people put comments in their > inline assembler too. :( Looks generally okay, but in order for it to be as simple (and hence understandable) as possible ... > --- a/xen/Makefile > +++ b/xen/Makefile > @@ -150,7 +150,7 @@ include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s > echo "#ifndef __ASM_OFFSETS_H__"; \ > echo "#define __ASM_OFFSETS_H__"; \ > echo ""; \ > - sed -ne "/^->/{s:^->\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 /* \3 */:; s:->::; p;}"; \ > + sed -ne "/==>/{s:^.*==>\(.*\)<==.*:\1:; s:^\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 /* \3 */:; p;}"; \ ... I think you should drop the ^ anchoring here, at least for the first expression (.* will match from the beginning of the string anyway). I also wonder whether, now that we're intending to make use of it elsewhere anyway, you shouldn't pass -r too, allowing all the escapes on the parentheses to be dropped. Jan
At 08:14 +0000 on 28 Mar (1395990898), Jan Beulich wrote: > >>> On 27.03.14 at 19:01, <tim@xen.org> wrote: > > The patch below works for me (at least as far as building > > asm-offsets.h on x86) by wrapping everything in a string. I did try > > just prefixing with '#' but clang 3.5 also strips the comments out. > > That seems unhelpful, since I know some people put comments in their > > inline assembler too. :( > > Looks generally okay, but in order for it to be as simple (and hence > understandable) as possible ... > > > --- a/xen/Makefile > > +++ b/xen/Makefile > > @@ -150,7 +150,7 @@ include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s > > echo "#ifndef __ASM_OFFSETS_H__"; \ > > echo "#define __ASM_OFFSETS_H__"; \ > > echo ""; \ > > - sed -ne "/^->/{s:^->\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 /* \3 */:; s:->::; p;}"; \ > > + sed -ne "/==>/{s:^.*==>\(.*\)<==.*:\1:; s:^\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 /* \3 */:; p;}"; \ > > ... I think you should drop the ^ anchoring here, at least for the first > expression (.* will match from the beginning of the string anyway). Ack, will do. > I also wonder whether, now that we're intending to make use of it > elsewhere anyway, you shouldn't pass -r too, allowing all the escapes > on the parentheses to be dropped. OK. Do you think it would be worth shuffling most of the creation of the #define into the C side too? I think we can do everything except the $ or # prefix of the immediate value. That would leave the RE looking something like this: sed -ner "/==>/{s:.*==>(.*)<==.*:\1:; s:[\$$#]::; p;}". Cheers, Tim.
>>> On 28.03.14 at 12:28, <tim@xen.org> wrote: > At 08:14 +0000 on 28 Mar (1395990898), Jan Beulich wrote: >> I also wonder whether, now that we're intending to make use of it >> elsewhere anyway, you shouldn't pass -r too, allowing all the escapes >> on the parentheses to be dropped. > > OK. Do you think it would be worth shuffling most of the creation of > the #define into the C side too? I think we can do everything except > the $ or # prefix of the immediate value. That would leave the RE looking > something like this: sed -ner "/==>/{s:.*==>(.*)<==.*:\1:; s:[\$$#]::; p;}". Yes, I think anything allowing this ugly sed expression to be reduced would be beneficial. Jan
Hi Tim, On 27/03/14 18:01, Tim Deegan wrote: > > The patch below works for me (at least as far as building > asm-offsets.h on x86) by wrapping everything in a string. I did try > just prefixing with '#' but clang 3.5 also strips the comments out. > That seems unhelpful, since I know some people put comments in their > inline assembler too. :( I'm able to build correctly x86 with your patch, and this patch (e.g #2) reverted. But for ARM ... it breaks in another place :( vfp.c:8:25: error: invalid operand for instruction v->arch.vfp.fpexc = READ_CP32(FPEXC); <inline asm>:1:6: note: instantiated into assembly here mrc p10, 7, r1, c8, c0, 0; ^ Coprocessor p10 (and p11) are used for Neon instruction are clang doesn't allow to use it directly. (http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131104/194170.html). Depends on which part of the ARM ARM you are reading, p10 and p11 should not be used directly with mrc/mcr instruction ... but gas accept it. I guess the best solution is to use directly the VFP instructions but it would mean to re-enable VFP at compile time in Xen (see http://www.gossamer-threads.com/lists/xen/devel/284653?do=post_view_threaded). Regards,
>>> On 29.03.14 at 23:55, <julien.grall@linaro.org> wrote: > But for ARM ... it breaks in another place :( > > vfp.c:8:25: error: invalid operand for instruction > v->arch.vfp.fpexc = READ_CP32(FPEXC); > <inline asm>:1:6: note: instantiated into assembly here > mrc p10, 7, r1, c8, c0, 0; > ^ > > Coprocessor p10 (and p11) are used for Neon instruction are clang > doesn't allow to use it directly. > (http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131104/194170.htm > l). > > Depends on which part of the ARM ARM you are reading, p10 and p11 should > not be used directly with mrc/mcr instruction ... but gas accept it. > > I guess the best solution is to use directly the VFP instructions but it > would mean to re-enable VFP at compile time in Xen (see > http://www.gossamer-threads.com/lists/xen/devel/284653?do=post_view_threaded) > . Or use .arch_extension around the instruction? Jan
On Sat, 2014-03-29 at 22:55 +0000, Julien Grall wrote: > Hi Tim, > > On 27/03/14 18:01, Tim Deegan wrote: > > > > The patch below works for me (at least as far as building > > asm-offsets.h on x86) by wrapping everything in a string. I did try > > just prefixing with '#' but clang 3.5 also strips the comments out. > > That seems unhelpful, since I know some people put comments in their > > inline assembler too. :( > > I'm able to build correctly x86 with your patch, and this patch (e.g #2) > reverted. > > But for ARM ... it breaks in another place :( > > vfp.c:8:25: error: invalid operand for instruction > v->arch.vfp.fpexc = READ_CP32(FPEXC); > <inline asm>:1:6: note: instantiated into assembly here > mrc p10, 7, r1, c8, c0, 0; > ^ > > Coprocessor p10 (and p11) are used for Neon instruction are clang > doesn't allow to use it directly. > (http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131104/194170.html). > > Depends on which part of the ARM ARM you are reading, p10 and p11 should > not be used directly with mrc/mcr instruction ... but gas accept it. > > I guess the best solution is to use directly the VFP instructions but it > would mean to re-enable VFP at compile time in Xen (see > http://www.gossamer-threads.com/lists/xen/devel/284653?do=post_view_threaded). Do the VFP instructions have different encodings? I thought this was an assembler mnemonic difference only. Ian.
On 04/01/2014 02:11 PM, Ian Campbell wrote: > On Sat, 2014-03-29 at 22:55 +0000, Julien Grall wrote: >> Hi Tim, >> >> On 27/03/14 18:01, Tim Deegan wrote: >>> >>> The patch below works for me (at least as far as building >>> asm-offsets.h on x86) by wrapping everything in a string. I did try >>> just prefixing with '#' but clang 3.5 also strips the comments out. >>> That seems unhelpful, since I know some people put comments in their >>> inline assembler too. :( >> >> I'm able to build correctly x86 with your patch, and this patch (e.g #2) >> reverted. >> >> But for ARM ... it breaks in another place :( >> >> vfp.c:8:25: error: invalid operand for instruction >> v->arch.vfp.fpexc = READ_CP32(FPEXC); >> <inline asm>:1:6: note: instantiated into assembly here >> mrc p10, 7, r1, c8, c0, 0; >> ^ >> >> Coprocessor p10 (and p11) are used for Neon instruction are clang >> doesn't allow to use it directly. >> (http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131104/194170.html). >> >> Depends on which part of the ARM ARM you are reading, p10 and p11 should >> not be used directly with mrc/mcr instruction ... but gas accept it. >> >> I guess the best solution is to use directly the VFP instructions but it >> would mean to re-enable VFP at compile time in Xen (see >> http://www.gossamer-threads.com/lists/xen/devel/284653?do=post_view_threaded). > > Do the VFP instructions have different encodings? I thought this was an > assembler mnemonic difference only. Strictly speaking it works only because vldm* as the same encoding as ldc*. But the processor will decode the instruction as vldm* (and execute as it is). LLVM assembly parser prevents the user to use ldc* in the assembly as specified by the ARM ARM.
On Tue, 2014-04-01 at 15:50 +0100, Julien Grall wrote: > On 04/01/2014 02:11 PM, Ian Campbell wrote: > > On Sat, 2014-03-29 at 22:55 +0000, Julien Grall wrote: > >> Hi Tim, > >> > >> On 27/03/14 18:01, Tim Deegan wrote: > >>> > >>> The patch below works for me (at least as far as building > >>> asm-offsets.h on x86) by wrapping everything in a string. I did try > >>> just prefixing with '#' but clang 3.5 also strips the comments out. > >>> That seems unhelpful, since I know some people put comments in their > >>> inline assembler too. :( > >> > >> I'm able to build correctly x86 with your patch, and this patch (e.g #2) > >> reverted. > >> > >> But for ARM ... it breaks in another place :( > >> > >> vfp.c:8:25: error: invalid operand for instruction > >> v->arch.vfp.fpexc = READ_CP32(FPEXC); > >> <inline asm>:1:6: note: instantiated into assembly here > >> mrc p10, 7, r1, c8, c0, 0; > >> ^ > >> > >> Coprocessor p10 (and p11) are used for Neon instruction are clang > >> doesn't allow to use it directly. > >> (http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131104/194170.html). > >> > >> Depends on which part of the ARM ARM you are reading, p10 and p11 should > >> not be used directly with mrc/mcr instruction ... but gas accept it. > >> > >> I guess the best solution is to use directly the VFP instructions but it > >> would mean to re-enable VFP at compile time in Xen (see > >> http://www.gossamer-threads.com/lists/xen/devel/284653?do=post_view_threaded). > > > > Do the VFP instructions have different encodings? I thought this was an > > assembler mnemonic difference only. > > Strictly speaking it works only because vldm* as the same encoding as > ldc*. But the processor will decode the instruction as vldm* (and > execute as it is). Are those the instructions in question though? The thing you quoted was about reading FPEXC which is an mrc cp instruction vs vmrs I think. They do indeed have identical encodings, so I'm not entirely sure what the big deal is about which assembler mnemonic gets used is, the process has no idea what we wrote in the source. > LLVM assembly parser prevents the user to use ldc* in the assembly as > specified by the ARM ARM.
On 04/01/2014 04:28 PM, Ian Campbell wrote: > On Tue, 2014-04-01 at 15:50 +0100, Julien Grall wrote: >> On 04/01/2014 02:11 PM, Ian Campbell wrote: >>> On Sat, 2014-03-29 at 22:55 +0000, Julien Grall wrote: >>>> Hi Tim, >>>> >>>> On 27/03/14 18:01, Tim Deegan wrote: >>>>> >>>>> The patch below works for me (at least as far as building >>>>> asm-offsets.h on x86) by wrapping everything in a string. I did try >>>>> just prefixing with '#' but clang 3.5 also strips the comments out. >>>>> That seems unhelpful, since I know some people put comments in their >>>>> inline assembler too. :( >>>> >>>> I'm able to build correctly x86 with your patch, and this patch (e.g #2) >>>> reverted. >>>> >>>> But for ARM ... it breaks in another place :( >>>> >>>> vfp.c:8:25: error: invalid operand for instruction >>>> v->arch.vfp.fpexc = READ_CP32(FPEXC); >>>> <inline asm>:1:6: note: instantiated into assembly here >>>> mrc p10, 7, r1, c8, c0, 0; >>>> ^ >>>> >>>> Coprocessor p10 (and p11) are used for Neon instruction are clang >>>> doesn't allow to use it directly. >>>> (http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131104/194170.html). >>>> >>>> Depends on which part of the ARM ARM you are reading, p10 and p11 should >>>> not be used directly with mrc/mcr instruction ... but gas accept it. >>>> >>>> I guess the best solution is to use directly the VFP instructions but it >>>> would mean to re-enable VFP at compile time in Xen (see >>>> http://www.gossamer-threads.com/lists/xen/devel/284653?do=post_view_threaded). >>> >>> Do the VFP instructions have different encodings? I thought this was an >>> assembler mnemonic difference only. >> >> Strictly speaking it works only because vldm* as the same encoding as >> ldc*. But the processor will decode the instruction as vldm* (and >> execute as it is). > > Are those the instructions in question though? The thing you quoted was > about reading FPEXC which is an mrc cp instruction vs vmrs I think. They > do indeed have identical encodings, so I'm not entirely sure what the > big deal is about which assembler mnemonic gets used is, the process has > no idea what we wrote in the source. Oh sorry, I took one in random. Every VFP instructions has the same issue. I will raise the bug to Clang. And see if why they choose this solution.
On Thu, 2014-04-03 at 18:07 +0200, Tim Deegan wrote: > Newer versions of clang attempt to parse inline assembler even when > not asked to assemble it. Wrap our not-for-assembly runes as strings > of the form ``.ascii "==>MAGIC RUNES<=="'' so clang doesn't choke on > them. > > While we're at it, assemble more of the final output line in the C > file, to make the sed expression shorter. > > Reported-by: Julien Grall <julien.grall@linaro.org> > Suggested-by: Jan Beulich <JBeulich@suse.com> > Signed-off-by: Tim Deegan <tim@xen.org> For ARM size, assuming you've build tested it: Acked-by: Ian Campbell <ian.campbell@citrix.com> Ian.
>>> On 03.04.14 at 18:07, <tim@xen.org> wrote: > Newer versions of clang attempt to parse inline assembler even when > not asked to assemble it. Wrap our not-for-assembly runes as strings > of the form ``.ascii "==>MAGIC RUNES<=="'' so clang doesn't choke on > them. > > While we're at it, assemble more of the final output line in the C > file, to make the sed expression shorter. > > Reported-by: Julien Grall <julien.grall@linaro.org> > Suggested-by: Jan Beulich <JBeulich@suse.com> > Signed-off-by: Tim Deegan <tim@xen.org> x86 side Acked-by: Jan Beulich <jbeulich@suse.com> Albeit considering the similarity (if not being fully identical) I wonder whether these definitions shouldn't be moved into a central place. Jan > --- > v2: make up the '#define' in the C source rather then in sed. > Not quite as neat as I'd hoped: need to match an extra space > to stop the second sed expression eating the # in #define. > > diff --git a/xen/Makefile b/xen/Makefile > index 79fa8f2..afbc962 100644 > --- a/xen/Makefile > +++ b/xen/Makefile > @@ -150,7 +150,7 @@ include/asm-$(TARGET_ARCH)/asm-offsets.h: > arch/$(TARGET_ARCH)/asm-offsets.s > echo "#ifndef __ASM_OFFSETS_H__"; \ > echo "#define __ASM_OFFSETS_H__"; \ > echo ""; \ > - sed -ne "/^->/{s:^->\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 /* \3 */:; > s:->::; p;}"; \ > + sed -rne "/==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}"; \ > echo ""; \ > echo "#endif") <$< >$@ > > diff --git a/xen/arch/arm/arm32/asm-offsets.c b/xen/arch/arm/arm32/asm-offsets.c > index ac628c0..cd1dff7 100644 > --- a/xen/arch/arm/arm32/asm-offsets.c > +++ b/xen/arch/arm/arm32/asm-offsets.c > @@ -13,11 +13,12 @@ > #include <asm/current.h> > #include <asm/procinfo.h> > > -#define DEFINE(_sym, _val) \ > - __asm__ __volatile__ ( "\n->" #_sym " %0 " #_val : : "i" (_val) ) > -#define BLANK() \ > - __asm__ __volatile__ ( "\n->" : : ) > -#define OFFSET(_sym, _str, _mem) \ > +#define DEFINE(_sym, _val) > \ > + asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \ > + : : "i" (_val) ) > +#define BLANK() > \ > + asm volatile ( "\n.ascii\"==><==\"" : : ) > +#define OFFSET(_sym, _str, _mem) > \ > DEFINE(_sym, offsetof(_str, _mem)); > > void __dummy__(void) > diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c > index d7572fa..a3ce816 100644 > --- a/xen/arch/arm/arm64/asm-offsets.c > +++ b/xen/arch/arm/arm64/asm-offsets.c > @@ -12,11 +12,12 @@ > #include <public/xen.h> > #include <asm/current.h> > > -#define DEFINE(_sym, _val) \ > - __asm__ __volatile__ ( "\n->" #_sym " %0 " #_val : : "i" (_val) ) > -#define BLANK() \ > - __asm__ __volatile__ ( "\n->" : : ) > -#define OFFSET(_sym, _str, _mem) \ > +#define DEFINE(_sym, _val) > \ > + asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \ > + : : "i" (_val) ) > +#define BLANK() > \ > + asm volatile ( "\n.ascii\"==><==\"" : : ) > +#define OFFSET(_sym, _str, _mem) > \ > DEFINE(_sym, offsetof(_str, _mem)); > > void __dummy__(void) > diff --git a/xen/arch/x86/x86_64/asm-offsets.c > b/xen/arch/x86/x86_64/asm-offsets.c > index b0098b3..9acb648 100644 > --- a/xen/arch/x86/x86_64/asm-offsets.c > +++ b/xen/arch/x86/x86_64/asm-offsets.c > @@ -14,11 +14,12 @@ > #include <asm/hardirq.h> > #include <xen/multiboot.h> > > -#define DEFINE(_sym, _val) \ > - __asm__ __volatile__ ( "\n->" #_sym " %0 " #_val : : "i" (_val) ) > -#define BLANK() \ > - __asm__ __volatile__ ( "\n->" : : ) > -#define OFFSET(_sym, _str, _mem) \ > +#define DEFINE(_sym, _val) > \ > + asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \ > + : : "i" (_val) ) > +#define BLANK() > \ > + asm volatile ( "\n.ascii\"==><==\"" : : ) > +#define OFFSET(_sym, _str, _mem) > \ > DEFINE(_sym, offsetof(_str, _mem)); > > void __dummy__(void)
Hi Tim, I will resend a second version of my clang series soon. Can I add this patch into it? Thanks, On 04/03/2014 05:07 PM, Tim Deegan wrote: > Newer versions of clang attempt to parse inline assembler even when > not asked to assemble it. Wrap our not-for-assembly runes as strings > of the form ``.ascii "==>MAGIC RUNES<=="'' so clang doesn't choke on > them. > > While we're at it, assemble more of the final output line in the C > file, to make the sed expression shorter. > > Reported-by: Julien Grall <julien.grall@linaro.org> > Suggested-by: Jan Beulich <JBeulich@suse.com> > Signed-off-by: Tim Deegan <tim@xen.org> > --- > v2: make up the '#define' in the C source rather then in sed. > Not quite as neat as I'd hoped: need to match an extra space > to stop the second sed expression eating the # in #define. > > diff --git a/xen/Makefile b/xen/Makefile > index 79fa8f2..afbc962 100644 > --- a/xen/Makefile > +++ b/xen/Makefile > @@ -150,7 +150,7 @@ include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s > echo "#ifndef __ASM_OFFSETS_H__"; \ > echo "#define __ASM_OFFSETS_H__"; \ > echo ""; \ > - sed -ne "/^->/{s:^->\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 /* \3 */:; s:->::; p;}"; \ > + sed -rne "/==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}"; \ > echo ""; \ > echo "#endif") <$< >$@ > > diff --git a/xen/arch/arm/arm32/asm-offsets.c b/xen/arch/arm/arm32/asm-offsets.c > index ac628c0..cd1dff7 100644 > --- a/xen/arch/arm/arm32/asm-offsets.c > +++ b/xen/arch/arm/arm32/asm-offsets.c > @@ -13,11 +13,12 @@ > #include <asm/current.h> > #include <asm/procinfo.h> > > -#define DEFINE(_sym, _val) \ > - __asm__ __volatile__ ( "\n->" #_sym " %0 " #_val : : "i" (_val) ) > -#define BLANK() \ > - __asm__ __volatile__ ( "\n->" : : ) > -#define OFFSET(_sym, _str, _mem) \ > +#define DEFINE(_sym, _val) \ > + asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \ > + : : "i" (_val) ) > +#define BLANK() \ > + asm volatile ( "\n.ascii\"==><==\"" : : ) > +#define OFFSET(_sym, _str, _mem) \ > DEFINE(_sym, offsetof(_str, _mem)); > > void __dummy__(void) > diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c > index d7572fa..a3ce816 100644 > --- a/xen/arch/arm/arm64/asm-offsets.c > +++ b/xen/arch/arm/arm64/asm-offsets.c > @@ -12,11 +12,12 @@ > #include <public/xen.h> > #include <asm/current.h> > > -#define DEFINE(_sym, _val) \ > - __asm__ __volatile__ ( "\n->" #_sym " %0 " #_val : : "i" (_val) ) > -#define BLANK() \ > - __asm__ __volatile__ ( "\n->" : : ) > -#define OFFSET(_sym, _str, _mem) \ > +#define DEFINE(_sym, _val) \ > + asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \ > + : : "i" (_val) ) > +#define BLANK() \ > + asm volatile ( "\n.ascii\"==><==\"" : : ) > +#define OFFSET(_sym, _str, _mem) \ > DEFINE(_sym, offsetof(_str, _mem)); > > void __dummy__(void) > diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c > index b0098b3..9acb648 100644 > --- a/xen/arch/x86/x86_64/asm-offsets.c > +++ b/xen/arch/x86/x86_64/asm-offsets.c > @@ -14,11 +14,12 @@ > #include <asm/hardirq.h> > #include <xen/multiboot.h> > > -#define DEFINE(_sym, _val) \ > - __asm__ __volatile__ ( "\n->" #_sym " %0 " #_val : : "i" (_val) ) > -#define BLANK() \ > - __asm__ __volatile__ ( "\n->" : : ) > -#define OFFSET(_sym, _str, _mem) \ > +#define DEFINE(_sym, _val) \ > + asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \ > + : : "i" (_val) ) > +#define BLANK() \ > + asm volatile ( "\n.ascii\"==><==\"" : : ) > +#define OFFSET(_sym, _str, _mem) \ > DEFINE(_sym, offsetof(_str, _mem)); > > void __dummy__(void) >
At 14:17 +0100 on 23 Apr (1398259077), Julien Grall wrote: > Hi Tim, > > I will resend a second version of my clang series soon. Can I add this > patch into it? Actually it's already acked and was just waiting for me to commit it, which I've just done. So, no need. :) Cheers, Tim.
On 04/24/2014 11:45 AM, Tim Deegan wrote: > At 14:17 +0100 on 23 Apr (1398259077), Julien Grall wrote: >> Hi Tim, >> >> I will resend a second version of my clang series soon. Can I add this >> patch into it? > > Actually it's already acked and was just waiting for me to commit it, > which I've just done. So, no need. :) Thanks! I will drop patch #2 of my series. Regards,
diff --git a/xen/Rules.mk b/xen/Rules.mk index 18fbd62..ce54926 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -74,6 +74,7 @@ AFLAGS-y += -D__ASSEMBLY__ -include $(BASEDIR)/include/xen/config # Clang's built-in assembler can't handle .code16/.code32/.code64 yet AFLAGS-$(clang) += -no-integrated-as +CFLAGS-$(clang) += -no-integrated-as ALL_OBJS := $(ALL_OBJS-y)
Clang 3.5 is trying to parse every assembly line in C file. The files asm-offsets.c are taking advantage of the inline assembly but doesn't produce valid assembly. x86_64/asm-offsets.c:26:5: error: unexpected token at start of statement OFFSET(UREGS_r15, struct cpu_user_regs, r15); ^ x86_64/asm-offsets.c:22:5: note: expanded from macro 'OFFSET' DEFINE(_sym, offsetof(_str, _mem)); ^ x86_64/asm-offsets.c:18:31: note: expanded from macro 'DEFINE' __asm__ __volatile__ ( "\n->" #_sym " %0 " #_val : : "i" (_val) ) ^ <inline asm>:2:1: note: instantiated into assembly here ->UREGS_r15 $0 offsetof(struct cpu_user_regs, r15) Signed-off-by: Julien Grall <julien.grall@linaro.org> Cc: Keir Fraser <keir@xen.org> --- xen/Rules.mk | 1 + 1 file changed, 1 insertion(+)