search

tum-ei-eda / etiss

Extendable Translating Instruction Set Simulator
https://tum-ei-eda.github.io/etiss/
Other
29 stars 36 forks source link

LLVM versions > 11 break compilation #87

Closed wysiwyng closed 3 years ago

wysiwyng commented 3 years ago

Commit 6470d4d601c77b65cd8ddf35514fcad9228779dc relaxed LLVM version checks to allow version 11 and up. This breaks at least on my system (Arch Linux) with LLVM version 12.0.1. A reason why the CI didn't catch this fault is that in Windows builds LLVM is not even used, and on Linux / Mac builds LLVM version 11.0.0 is manually installed.

A better fix for the CI failure the aforementioned commit tried to fix would be to force installation of LLVM 11 on windows: choco install llvm --version=11.0.0 --allow-downgrade

rafzi commented 3 years ago

Sorry for that! I trusted the CI too much on that one, but like you mentioned, it does not use LLVM:

https://github.com/tum-ei-eda/etiss/runs/3859384773#step:9:34

This is not expected, because we do support LLVMJIT on Windows. I could not track back to when this first stopped working, since older logs are not kept by github.

Another strange behavior is that ETISS switches the JIT engine even if we explicitly specify one. This is bad for testing.

https://github.com/tum-ei-eda/etiss/blob/master/.github/workflows/ci.yml#L137

wysiwyng commented 3 years ago

This is not expected, because we do support LLVMJIT on Windows.

I would expect this line: https://github.com/tum-ei-eda/etiss/blob/2da3eca706c4138046743957cfd0f1a473b4d6c3/.github/workflows/ci.yml#L68 is responsible for LLVM not working, as chocolatey most definitely does not install LLVM into that directory.

Another strange behavior is that ETISS switches the JIT engine even if we explicitly specify one. This is bad for testing.

This is explained rather quickly, ETISS switches to its default engine (that is TCC afaik) when the specified one doesn't work. Which admittedly is not ideal as you say, maybe an error would be more suitable in this case.

lukasauer commented 3 years ago

I noticed the same today. Just wanted to add that the compile issues are caused by this LLVM change, which changes the function signature of overrideFileContents().

/etiss/JITImpl/LLVM/src/LLVMJIT.cpp: In member function ‘virtual void* etiss::LLVMJIT::translate(std::string, std::set<std::__cxx11::basic_string<char> >, std::set<std::__cxx11::basic_string<char> >, std::set<std::__cxx11::basic_string<char> >, std::string&, bool)’:
/etiss/JITImpl/LLVM/src/LLVMJIT.cpp:265:47: error: no matching function for call to ‘clang::SourceManager::overrideFileContents(const clang::FileEntry*, std::unique_ptr<llvm::MemoryBuffer>::pointer, bool)’
  265 |     CI.getSourceManager().overrideFileContents(
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
  266 |         CI.getFileManager().getVirtualFile("/etiss_llvm_clang_memory_mapped_file.c", buffer->getBufferSize(), 0),
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  267 |         buffer.get(), true);
      |         ~~~~~~~~~~~~~~~~~~~
rafzi commented 3 years ago

The change to allow version 12 was reverted and the suggested allow-downgrade is used now. The other issues are tracked in #92 and #93