[Mesa-dev] [PATCH 2/9] gallivm: Use LLVMSetTarget.

[Emil Velikov]

On 18 April 2016 at 14:16, Jose Fonseca <jfonseca at vmware.com> wrote:
> On 18/04/16 13:27, Emil Velikov wrote:
>>
>> Hi Jose,
>>
>> On 18 April 2016 at 10:14, Jose Fonseca <jfonseca at vmware.com> wrote:
>>>
>>> Instead of LLVM C++ interfaces.
>>> ---
>>>   src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 8 +++++---
>>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>>> b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>>> index c1e262b..37e2f08 100644
>>> --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>>> @@ -519,9 +519,11 @@
>>> lp_build_create_jit_compiler_for_module(LLVMExecutionEngineRef *OutJIT,
>>>          /*
>>>           * MCJIT works on Windows, but currently only through ELF object
>>> format.
>>>           */
>>> -       std::string targetTriple = llvm::sys::getProcessTriple();
>>> -       targetTriple.append("-elf");
>>> -       unwrap(M)->setTargetTriple(targetTriple);
>>> +#  ifdef _WIN64
>>> +       LLVMSetTarget(M, "x86_64-pc-win32-elf");
>>> +#  else
>>> +       LLVMSetTarget(M, "i686-pc-win32-elf");
>>> +#  endif
>>
>>
>> I've noticed that you're using LLVM_HOST_TRIPLE in patch 7/9. Wouldn't
>> it be better to use it here as well ?
>>
>> +       LLVMSetTarget(M, LLVM_HOST_TRIPLE "-elf");
>
>
> Thanks for taking a look.
>
> It's a good remark.
>
> Surprisingly LLVM uses different LLVM_HOST_TRIPLE for MinGW/MSVC:
>
> $ grep LLVM_HOST_TRIPLE */llvm-*/include
> mingw32/llvm-3.3.1/include/llvm/Config/config.h:#define LLVM_HOST_TRIPLE
> "i686-pc-mingw32"
> mingw32/llvm-3.4.1/include/llvm/Config/config.h:#define LLVM_HOST_TRIPLE
> "i686-pc-mingw32"
> mingw64/llvm-3.3.1/include/llvm/Config/config.h:#define LLVM_HOST_TRIPLE
> "x86_64-w64-mingw32"
> mingw64/llvm-3.4.1/include/llvm/Config/config.h:#define LLVM_HOST_TRIPLE
> "x86_64-w64-mingw32"
> msvc32/llvm-3.3.1/include/llvm/Config/config.h:#define LLVM_HOST_TRIPLE
> "i686-pc-win32"
> msvc32/llvm-3.4.1/include/llvm/Config/config.h:#define LLVM_HOST_TRIPLE
> "i686-pc-win32"
> msvc64/llvm-3.3.1/include/llvm/Config/config.h:#define LLVM_HOST_TRIPLE
> "x86_64-pc-win32"
> msvc64/llvm-3.4.1/include/llvm/Config/config.h:#define LLVM_HOST_TRIPLE
> "x86_64-pc-win32"
>

(Very mildly) related
There's also the missing header for 'other' wordsize configs.
llvm/Config/llvm-config.h does mention both -32 and -64 headers, but
only one is shipped. Guessing that just like above it's a intentional
design decision or a bug ;-)


>> That aside I'm really glad to see mesa (modulo swr) no longer using
>> the unstable LLVM C++ API.
>
>
> Still not quite there, but yes that's indeed the hope.
>
>> Perhaps at some point we could port these
>>
>> to normal C and make gallivm 'C++ free'  ;-)
>
>
> There's no much use of C++ left indeed, and at some point might as well
> remove it completely.
>
Glad we're on the same page.


> For my part, my personal goal is to eliminate src/gallium/auxiliary/rtasm in
> the medium term.  And I think should remove src/mesa/x86/rtasm too.
>
Sounds like a plan. This might cause some minor perf degradation, as
we nuke the translate_sse2 path, but I guess one can benchmark that in
due time. On the other hand, if we mandate LLVM for those paths we
might end up 'forcing' it on driver which don't use it yet - nouveau,
ilo, freedreno, vc4, virgl...

Hopefully I didn't open a (the) can of worms with that last statement.

-Emil