[Mesa-dev] [PATCH v2 05/14] swr: [rasterizer jitter] cleanup supporting different llvm versions

[Rowley, Timothy O]

> On Jun 22, 2016, at 1:52 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> 
> On 20 June 2016 at 22:36, Tim Rowley <timothy.o.rowley at intel.com> wrote:
>> ---
>> .../drivers/swr/rasterizer/jitter/JitManager.cpp   |  9 +++++--
>> .../drivers/swr/rasterizer/jitter/JitManager.h     |  7 ++++-
>> .../drivers/swr/rasterizer/jitter/blend_jit.cpp    |  8 +-----
>> .../drivers/swr/rasterizer/jitter/builder_misc.cpp | 31 +++++++++++++++++++---
>> .../drivers/swr/rasterizer/jitter/builder_misc.h   |  6 +++++
>> .../drivers/swr/rasterizer/jitter/fetch_jit.cpp    | 15 ++---------
>> .../jitter/scripts/gen_llvm_ir_macros.py           | 24 ++++++++++++++++-
>> .../swr/rasterizer/jitter/streamout_jit.cpp        |  7 +----
>> 8 files changed, 73 insertions(+), 34 deletions(-)
>> 
>> diff --git a/src/gallium/drivers/swr/rasterizer/jitter/JitManager.cpp b/src/gallium/drivers/swr/rasterizer/jitter/JitManager.cpp
>> index 4bbd9ad..6e00a70 100644
>> --- a/src/gallium/drivers/swr/rasterizer/jitter/JitManager.cpp
>> +++ b/src/gallium/drivers/swr/rasterizer/jitter/JitManager.cpp
>> @@ -35,11 +35,13 @@
>> #include "JitManager.h"
>> #include "fetch_jit.h"
>> 
>> +#pragma push_macro("DEBUG")
>> +#undef DEBUG
>> +
>> #if defined(_WIN32)
>> #include "llvm/ADT/Triple.h"
>> #endif
>> #include "llvm/IR/Function.h"
>> -#include "llvm/Support/DynamicLibrary.h"
>> 
>> #include "llvm/Support/MemoryBuffer.h"
>> #include "llvm/Support/SourceMgr.h"
>> @@ -53,6 +55,8 @@
>> #include "llvm/ExecutionEngine/JITEventListener.h"
>> #endif
>> 
>> +#pragma pop_macro("DEBUG")
>> +
> I'm afraid that these still are still off - they should be wrapped in
> "if HAVE_LLVM >= 0x0307 ... endif". Plus the ones in JitManager.h
> really want a similar treatment.

Any reason to avoid the push/pop on older LLVM?  Saves things from becoming too messy with preprocessor directives.

> Mildly related bugs/cleanups:
> - There's a few cases of _DEBUG which should (?) be replaced with ifndef NDEBUG

Ok, I can address this in another patch.

> - swr uses both mesa and LLVM provided version macros. Please stick to one.
> If the latter is reliable (available all the way to min. supported
> LLVM version) and can be used in both C and C++ sources I'm inclined
> to just use it everywhere in mesa and drop out local macros…

Are you referring to the HAVE_LLVM macro?  I can remove the conditional definition of this from swr (since Mesa provides the definition).

-Tim