Thank you for the review. Comments inline.
> -----Original Message-----
> From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
> Sent: Tuesday, November 8, 2016 7:52 AM
> To: Kyriazis, George <george.kyria...@intel.com>; Jose Fonseca
> <jfons...@vmware.com>
> Cc: ML mesa-dev <mesa-dev@lists.freedesktop.org>
> Subject: Re: [Mesa-dev] [PATCH 1/3] gallium/scons: OpenSWR Windows
> support
>
> Hi George,
>
> For Scons changes please keep Jose Fonseca in the loop.
>
> On 7 November 2016 at 22:32, George Kyriazis <george.kyria...@intel.com>
> wrote:
> > - Added code to create screen and handle swaps in libgl_gdi.c
> > - Added call to swr SConscript
> > - included llvm 3.9 support for scons (windows swr only support 3.9 and
> > later)
> If that's the case building SWR with earlier one should error out ?
> Then again, here you reference gallium/drivers/swr/
>
Yes, SWR will only work on windows for 3.9 and above.
> > - include -DHAVE_SWR to subdirs that need it
> >
> As the above indicates here you have multiple independent changes.
> Please do _not_ mix those into a single patch.
>
I'll resend v2 of the patches with a different breakdown.
Additional comments on your review of patch 3/3.
Thanks,
George
>
> > To buils SWR on windows, use "scons swr libgl-gdi"
> > ---
> > scons/llvm.py | 21 +++++++++++++++++++--
> > src/gallium/SConscript | 1 +
> > src/gallium/targets/libgl-gdi/SConscript | 4 ++++
> > src/gallium/targets/libgl-gdi/libgl_gdi.c | 28
> > +++++++++++++++++++++++----- src/gallium/targets/libgl-xlib/SConscript
> | 4 ++++
> > src/gallium/targets/osmesa/SConscript | 4 ++++
> > 6 files changed, 55 insertions(+), 7 deletions(-)
> >
> > diff --git a/scons/llvm.py b/scons/llvm.py index 1fc8a3f..977e47a
> > 100644
> > --- a/scons/llvm.py
> > +++ b/scons/llvm.py
> > @@ -106,7 +106,24 @@ def generate(env):
> > ])
> > env.Prepend(LIBPATH = [os.path.join(llvm_dir, 'lib')])
> > # LIBS should match the output of `llvm-config --libs engine mcjit
> bitwriter x86asmprinter`
> > - if llvm_version >= distutils.version.LooseVersion('3.7'):
> > + if llvm_version >= distutils.version.LooseVersion('3.9'):
> > + env.Prepend(LIBS = [
> > + 'LLVMX86Disassembler', 'LLVMX86AsmParser',
> > + 'LLVMX86CodeGen', 'LLVMSelectionDAG', 'LLVMAsmPrinter',
> > + 'LLVMDebugInfoCodeView', 'LLVMCodeGen',
> > + 'LLVMScalarOpts', 'LLVMInstCombine',
> > + 'LLVMInstrumentation', 'LLVMTransformUtils',
> > + 'LLVMBitWriter', 'LLVMX86Desc',
> > + 'LLVMMCDisassembler', 'LLVMX86Info',
> > + 'LLVMX86AsmPrinter', 'LLVMX86Utils',
> > + 'LLVMMCJIT', 'LLVMExecutionEngine', 'LLVMTarget',
> > + 'LLVMAnalysis', 'LLVMProfileData',
> > + 'LLVMRuntimeDyld', 'LLVMObject', 'LLVMMCParser',
> > + 'LLVMBitReader', 'LLVMMC', 'LLVMCore',
> > + 'LLVMSupport',
> > + 'LLVMIRReader', 'LLVMASMParser'
> > + ])
> LLVM 3.9 support. cc: mesa-stable (if Jose/Brian are up for it).
>
> > + elif llvm_version >= distutils.version.LooseVersion('3.7'):
> > env.Prepend(LIBS = [
> > 'LLVMBitWriter', 'LLVMX86Disassembler', 'LLVMX86AsmParser',
> > 'LLVMX86CodeGen', 'LLVMSelectionDAG',
> > 'LLVMAsmPrinter', @@ -203,7 +220,7 @@ def generate(env):
> > if '-fno-rtti' in cxxflags:
> > env.Append(CXXFLAGS = ['-fno-rtti'])
> >
> > - components = ['engine', 'mcjit', 'bitwriter', 'x86asmprinter',
> 'mcdisassembler']
> > + components = ['engine', 'mcjit', 'bitwriter',
> > + 'x86asmprinter', 'mcdisassembler', 'irreader']
> Standalone bugfix. Cc: mesa-stable ?
>
>
> > +++ b/src/gallium/SConscript
>
> > + 'drivers/swr/SConscript',
> This file is only introduced with 3/3. Which means that you've added scons
> support which is broken - please don't do that.
>
>
> > +++ b/src/gallium/targets/libgl-gdi/SConscript
> > +++ b/src/gallium/targets/libgl-gdi/libgl_gdi.c
> > +++ b/src/gallium/targets/libgl-xlib/SConscript
> > +++ b/src/gallium/targets/osmesa/SConscript
>
> Couple of ideas how to split these. Or anything else that comes to mind on
> your end.
>
> A)
> Patch 1
> src/gallium/SConscript
> src/gallium/targets/libgl-gdi/SConscript
> src/gallium/targets/libgl-gdi/libgl_gdi.c
> Patch 2
> src/gallium/targets/libgl-xlib/SConscript
> src/gallium/targets/osmesa/SConscript
>
> B)
> Patch 1
> src/gallium/targets/libgl-gdi/libgl_gdi.c
> Patch 2
> src/gallium/SConscript
> src/gallium/targets/libgl-gdi/SConscript
> src/gallium/targets/libgl-xlib/SConscript
> src/gallium/targets/osmesa/SConscript
>
>
> Thanks
> Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
