The automatic header transformation is already done. What is not done is the preparation of a minimal patch.
Eddie On 1/9/11 6:46 PM, Philip Prindeville wrote: > On 1/9/11 6:42 PM, Eddie Kohler wrote: >> On 1/9/11 6:09 PM, Philip Prindeville wrote: >>> Are we sure that the "::" edit is still required? It seems to me that >>> recent compilers are able to handle "::" in asm(...) sequences without >>> modification. >> >> Since automatic header transformation is required anyway, we do :: >> along with the other stuff. > > That's a big change, and I'm thinking about smaller steps we could do in > the meantime. > >> >>> Can we move to a minimum compiler version that supports this? >> >> 0. Probably not. >> 1. I think asm(...::...) support was added quite recently and people >> compile Click with old compilers on embedded targets. > > Not always. OpenWRT is up to gcc 4.5.1. > > And sometimes, you have to move to the newer compilers to get support > for the enhanced instruction sets or the appropriate scheduling for > current stepping rates. > > > >> 2. Given automatic header transformation why would you care? > > Plan B, in case one is needed. > >> 3. If you have a specific patch or correct version number do send it >> for further discussion. > > I'll try to figure out what version of gcc the fix appeared in. > > >> >> Eddie >> >> >>> Thanks. >>> >>> -Philip >>> >>> On 1/9/11 10:38 AM, Eddie Kohler wrote: >>>> Philip, just to speak up. >>>> >>>> The most important outstanding project for Click is exactly minimizing >>>> patch size. Any help is appreciated! >>>> >>>> The first step is to correctly understand the problem. >>>> >>>> > I still don't get this. Even in a .cpp file, you can have >>>> > >>>> > c++ stuff >>>> > ... >>>> > extern "C" { >>>> > #include "c-header-with-double-colon-asms.h" >>>> > } >>>> > ... >>>> > >>>> > and the compiler should be fine with that. >>>> > >>>> > If not, then the compiler is broken. >>>> >>>> Not quite. As Joonwoo points out "extern "C" {}" doesn't tell the C++ >>>> compiler to stop being a C++ compiler. What is between the braces must >>>> still be correct C++: no use of C++ keywords as variables, no use of >>>> C++ operators like ::, and so forth. This is the main challenge of >>>> Click patching. Since Click's kernel module integrates with Linux, it >>>> must include Linux's header files; and therefore we must change those >>>> header files to be equivalent, but valid, C++ header files. >>>> >>>> Our patches have used a semi-mechanical process to make a bunch of >>>> syntactic changes (like :: => : : in asm statements), and then as you >>>> point out include 1000 lines of real changes. >>>> >>>> The way forward is "patchless". We have written a perl script that >>>> automates the semi-mechanical part. The script, >>>> linuxmodule/fixincludes.pl, generates new, C++-includable header files >>>> as part of the build process. These header files are equivalent to the >>>> Linux header files but safe for C++ compilers we care about. >>>> >>>> Unfortunately fixincludes.pl is not totally trivial and we have had to >>>> update it for new kernel releases. You can look at it to see what it >>>> does. Patches welcome. >>>> >>>> The goal of the patchless project is to make a version of Click that >>>> can be loaded into an UNPATCHED Linux kernel. This would make life a >>>> lot easier for many of our users. However, it's not going to achieve >>>> great performance. Patchless doesn't support polling, for example. And >>>> it's not clear that we can even support total-patchless Click on more >>>> recent versions of Linux, which have removed some of the hooks on >>>> which we relied. >>>> >>>> What we HAVE NOT done is separate out the 1000 lines of "real" >>>> changes. If you wanted to help with that process it would be lovely. >>>> Joonwoo, Cliff, and others have discussed some changes on the list to >>>> support newer kernels; I owe responses to them too. >>>> >>>> Eddie >>>> >>>> >>>> On 1/4/11 10:58 PM, Philip Prindeville wrote: >>>>> Ok, I just went back and looked at "click/cxxprotect.h" which >>>>> includes redefinition for C++ keywords like "new" and "virtual". >>>>> >>>>> So has anyone tried to compile Click with minimal patching (i.e. just >>>>> what is necessary to add the required instrumentation and hooks) with >>>>> a recent version of gcc (say 4.3.0 or later)? >>>>> >>>>> >>>>> >>>>> On 1/4/11 7:59 PM, Philip Prindeville wrote: >>>>>> Ok, so it seems to me that the solution is twofold: >>>>>> >>>>>> (1) require a reasonably recent version of gcc; >>>>>> (2) In places where the 'new' keyword is used in header files for >>>>>> inlines, etc. do: >>>>>> >>>>>> #define new __new >>>>>> #include<linux/list.h> >>>>>> #undef new >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> On 1/4/11 6:39 PM, Joonwoo Park wrote: >>>>>>> Hi, >>>>>>> >>>>>>> I don't think 'extern "C"' made any difference. If you were able to >>>>>>> compile your example you should be able to compile even without >>>>>>> 'extern "C"' from your example. >>>>>>> Don't remember exact gcc version but IIRC earlier version of g++ >>>>>>> parses '::' as namespace keyword always. To support old versions, >>>>>>> patching '::' was necessary. >>>>>>> >>>>>>> Joonwoo >>>>>>> >>>>>>> On Tue, Jan 4, 2011 at 5:35 PM, Philip Prindeville >>>>>>> <philipp_s...@redfish-solutions.com> wrote: >>>>>>>> On 1/4/11 12:25 PM, Joonwoo Park wrote: >>>>>>>>> 1 #include<iostream> >>>>>>>>> 2 >>>>>>>>> 3 extern "C" >>>>>>>>> 4 { >>>>>>>>> 5 struct keyword { >>>>>>>>> 6 int new; >>>>>>>>> 7 }; >>>>>>>>> 8 } >>>>>>>>> 9 >>>>>>>>> 10 int main() >>>>>>>>> 11 { >>>>>>>>> 12 return 0; >>>>>>>>> 13 } >>>>>>>> I took that and modified it as: >>>>>>>> >>>>>>>> #include<iostream> >>>>>>>> >>>>>>>> extern "C" >>>>>>>> { >>>>>>>> #include<sys/types.h> >>>>>>>> >>>>>>>> void load_ldt(int32_t ldt) >>>>>>>> { >>>>>>>> asm volatile("lldt %0"::"m" (ldt)); >>>>>>>> } >>>>>>>> } >>>>>>>> >>>>>>>> int main() >>>>>>>> { >>>>>>>> return 0; >>>>>>>> } >>>>>>>> >>>>>>>> >>>>>>>> and it compiled fine. >>>>>>>> >>>>>>>> So that solves part of the problem. >>>>>>>> >>>>>>>> -Philip >>>>>>>> >>>>>>>> >>>>>>>> >>>>>> >>> > _______________________________________________ click mailing list click@amsterdam.lcs.mit.edu https://amsterdam.lcs.mit.edu/mailman/listinfo/click