On 11/6/14, Tobias Grosser <tob...@grosser.es> wrote: > On 06.11.2014 11:15, Richard Biener wrote: >> On 11/6/14, Tobias Grosser <tob...@grosser.es> wrote: >>> On 06.11.2014 10:05, Richard Biener wrote: >>>> On Thu, Nov 6, 2014 at 8:02 AM, Tobias Grosser <tob...@grosser.es> >>>> wrote: >>>>> On 06.11.2014 07:04, Roman Gareev wrote: >>>>>>> >>>>>>> CLooG is not necessarily needed. You can run graphite just with ISL. >>>>>>> The >>>>>>> main reason that ISL code generation is not enabled by default is >>>>>>> that >>>>>>> we >>>>>>> did not yet get extensive testing and it was unclear who will have >>>>>>> the >>>>>>> time >>>>>>> to fix possible bugs. >>>>>> >>>>>> >>>>>> Could you please advise me which test suites should be used to make >>>>>> performance comparison between CLooG and ISL generator? (I would like >>>>>> to do this, even though the old generator is removed). >>>>> >>>>> >>>>> I do not have specific advices. You can use various open source >>>>> benchmarks >>>>> e.g. the LLVM test suite or, if you have access, you could run SPEC or >>>>> something. >>>>> >>>>>>> @Mircae, Roman: Would you have time to help with bug-fixing if we do >>>>>>> the >>>>>>> switch now? (I am happy to review patches and give advice, but can >>>>>>> not >>>>>>> do >>>>>>> the full move myself) >>>>>> >>>>>> >>>>>> I could find time for this. What do you mean by ‘switch’? If I’m not >>>>>> mistaken, ISL generator is already used by default. Should we remove >>>>>> support of CLooG generator and all files related to it? >>>>> >>>>> >>>>> Wow, I must really have been sleeping (or just forgetting). The switch >>>>> already happened. This is amazing. >>>>> >>>>> As the ISL code generator has been default since a while and we did >>>>> not >>>>> get >>>>> many bug reports, the actual switch seems to have worked well. We >>>>> could >>>>> probably still need some testing, but in this case it is most likely >>>>> time >>>>> to >>>>> drop the CLooG support entirely. Are you interested to provide the >>>>> relevant >>>>> patches? >>>>> >>>>> Also, as Tobias suggested we should raise the minimal supported isl >>>>> level >>>>> to >>>>> 0.14 to be sure PR 62289 is fixed. >>>> >>>> As I am testing with system isl/cloog that would be unfortunate as it >>>> means >>>> I'd either drop testing graphite for 4.8 and 4.9 or for 5.0 ... AFAIK >>>> ISL >>>> 12.x and 13+ cannot co-exist in the system due to include file >>>> conflicts. >>> >>> Sven, >>> >>> is there any chance we can add the deprecated isl_int includes back into >>> isl 0.14.1. This would unblock the testing and we could remove them as >>> soon as gcc 4.8/4.9 has been phased out. >>> >>> This would also fix my polly code coverage tests which do not work since >>> isl_int has been moved into different header files, as ubuntu does not >>> want to update isl as long as such an update breaks gcc. >> >> Ah, so it's still there? > > Yes, we just need to include some additional header files. > > Besides isl/aff.h we now also would need isl/deprecated/aff.h > >> Maybe we can simply add some configury to detect >> its location and fix build with a patch for 4.8 and 4.9? > > Is there maybe even >> a preprocessor macro one can check that newer ISL provide that can tell >> us where to look for isl_int? > > We could detect the need to include these files based on isl's version > macros. > > So you suggest that adding conditional includes would allow us to move > forward and would be part of some of the next gcc point releases. This > would be perfect.
Yes. Please make sure - if you produce a patch - that it works with both inline copy of ISL and system installed one. Thanks, Richard. > Tobias > > > > > >
