[Mesa-dev] [PATCH v3 04/25] configure.ac: Move oCL checks out of LLVM version check

[Tobias Droste]

Am Mittwoch, 12. Oktober 2016, 20:28:03 CEST schrieb Emil Velikov:
> On 12 October 2016 at 19:58, Tobias Droste <tdroste at gmx.de> wrote:
> > Am Mittwoch, 12. Oktober 2016, 19:51:21 CEST schrieb Emil Velikov:
> >> On 12 October 2016 at 19:04, Tobias Droste <tdroste at gmx.de> wrote:
> >> > Am Mittwoch, 12. Oktober 2016, 09:56:39 CEST schrieb Emil Velikov:
> >> >> >  fi
> >> >> > 
> >> >> > +if test "x$enable_opencl" = xyes; then
> >> >> > +    llvm_check_version_for "3" "6" "0" "opencl"
> >> >> > +
> >> >> > +    LLVM_COMPONENTS="${LLVM_COMPONENTS} all-targets ipo linker
> >> >> > instrumentation" +    LLVM_COMPONENTS="${LLVM_COMPONENTS} irreader
> >> >> > option
> >> >> > objcarcopts profiledata" +fi
> >> >> > +
> >> >> > +dnl Check for Clang internal headers
> >> >> > +if test "x$enable_opencl" = xyes; then
> >> >> 
> >> >> Nit: drop the second if test, yet preserve the comment ?
> >> >> Disclaimer: haven't looked if later patches depend on the split.
> >> > 
> >> > This is a "just move" patch, that's why I didn't change anything.
> >> > But this whole section will be changed later (Patch 11) so it actually
> >> > doesn't matter.
> >> 
> >> Patch 11... this hunks get moved once here and a second time in there.
> >> Just move it to the "top" from the start, fold the conditional and as
> >> you do further movement keep it in the same block ?
> >> 
> >> Sure I suggested to keep things separate, but it sounds like we got
> >> from one end/extreme to the other.
> >> Emil
> > 
> > No at this point in time it sadly has to be below the other gallium stuff,
> > because it uses LLVM variables but I need it outside the LLVM config stuff
> > to have this function later without any code that throws errors.
> > 
> > (Forgot the mailing list again)
> > 
> > PS:
> > This is the reason I didn't want to split this stuff and have it build
> > correctly. There's a lot of stuff that needs to be in the right order to
> > work. That's also the reason the oCL stuff is here and not where it
> > actually belongs!
> 
> In general - divide and concur. If the latter isn't working out the
> former needs refinement.
> 
> I'm starting to wonder if I shouldn't give it a bash myself rather
> than nitpicking like an old bat ?

I don't mind the feedback, so no worries there. But feel free to do so :-)
You can take whatever you want from my series and try to make it look better.

I just want to say that it looks easier than it actually is ;-)
Btw. some of the patches are also 2 patches instead of 1, because git 
sometimes isn't really that good in providing understandable diffs.

If you actually do try, give me a hint, so I can stop trying :-D

> 
> -Emil