harvard-acc / LLVM-Tracer

An LLVM pass to profile dynamic LLVM IR instructions and runtime values
Other
135 stars 35 forks source link

Allow cmake to get the LLVM recommended version from the environment #11

Closed z-a-f closed 8 years ago

z-a-f commented 8 years ago

In the TracerConfig.cmake LLVM version is checked as if(NOT DEFINED LLVM_RECOMMEND_VERSION). I believe the correct line should say if(NOT DEFINED ENV{LLVM_RECOMMEND_VERSION}). cmake pipermail

That way one can have export LLVM_RECOMMEND_VERSION="3.x" in the environment setup

ysshao commented 8 years ago

hi @zafartahirov , is the issue resolved?

z-a-f commented 8 years ago

It is partially resolved on a virtual machine. I will work on it closer this week - will ping back once I have something more substantial to say :)

z-a-f commented 8 years ago

Yes the issue is resolved, I still suggested some more configurability regarding the llvm version. As of now the ticket could be closed :)

ysshao commented 8 years ago

Great! Would be great if you can share a bit more about the issues that you run into while configuring the LLVM-Tracer and what you did to get it work. Chia-Wei (@rgly) helped us set up most of the cmake scripts here (Thanks!). Maybe he can also help out here.

rgly commented 8 years ago

I like the idea of getting the LLVM version from environment variables. But for testing purpose, some guys like me may run multiple LLVM-Tracer builds at the same time with different LLVM version. I suggest to check CMake variables first, then check environment variables.

Something like this?

if(NOT DEFINED LLVM_RECOMMEND_VERSION)
  if(NOT DEFINED ENV{LLVM_RECOMMEND_VERSION} )
      SET(LLVM_RECOMMEND_VERSION 3.4)
  else()
      SET(LLVM_RECOMMEND_VERSION   $ENV{LLVM_RECOMMEND_VERSION} )
  endif()
endif()
z-a-f commented 8 years ago

Yeah, currently I have it setup as

if(DEFINED LLVM_RECOMMEND_VERSION)
  message("LLVM_RECOMMEND_VERSION is defaulted to: ${LLVM_RECOMMEND_VERSION}")
elseif(DEFINED ENV{LLVM_RECOMMEND_VERSION})
  SET(LLVM_RECOMMEND_VERSION $ENV{LLVM_RECOMMEND_VERSION})
else()
  SET(LLVM_RECOMMEND_VERSION 3.4)
endif()

I am trying to use LLVM_RECOMMEND_VERSION as a first statement in the conditional in case someone decides to run the tracer from the terminal directly using -D flag. I am not requesting a pull request because there are still a ton of things I need to learn :smile: