Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tiny improvement #1093

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

tiny improvement #1093

wants to merge 2 commits into from

Conversation

joeyjiaojg
Copy link
Contributor

@joeyjiaojg joeyjiaojg commented May 16, 2023

  1. pass JOBS to build.sh
  2. pass COMPILE_FLAGS to cmake config

build.sh Outdated Show resolved Hide resolved
build.sh Outdated Show resolved Hide resolved
svf/include/Util/Annotator.h Outdated Show resolved Hide resolved
Copy link
Contributor Author

@joeyjiaojg joeyjiaojg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated, please check again?

build.sh Outdated Show resolved Hide resolved
build.sh Outdated Show resolved Hide resolved
@joeyjiaojg joeyjiaojg force-pushed the master branch 3 times, most recently from 10fdd04 to 7660ea6 Compare May 16, 2023 06:01
Copy link
Contributor Author

@joeyjiaojg joeyjiaojg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, pls review again.

build.sh Outdated
@@ -6,11 +6,13 @@
# Dependencies include: build-essential libncurses5 libncurses-dev cmake zlib1g-dev
set -e # exit on first error

jobs=4
jobs=$(echo $(nproc) || echo $(sysctl -n hw.physicalcpu))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make it one line rather than a if condition

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be faster when you were trying to build svf using more than 4 threads? I think we can leave it as 4 and people can simply adjust it by themselves

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much faster when I use blade with higher jobs.

build.sh Outdated Show resolved Hide resolved
build.sh Show resolved Hide resolved
@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Merging #1093 (f723d9d) into master (984e759) will not change coverage.
The diff coverage is n/a.

❗ Current head f723d9d differs from pull request most recent head 3a2f707. Consider uploading reports for the commit 3a2f707 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1093   +/-   ##
=======================================
  Coverage   64.56%   64.56%           
=======================================
  Files         222      222           
  Lines       23518    23518           
=======================================
  Hits        15184    15184           
  Misses       8334     8334           

build.sh Outdated Show resolved Hide resolved
@joeyjiaojg joeyjiaojg force-pushed the master branch 2 times, most recently from 46333d1 to 23b8c01 Compare May 18, 2023 06:53
@joeyjiaojg
Copy link
Contributor Author

The new patch should be better now.

@joeyjiaojg
Copy link
Contributor Author

CMake Error at CMakeLists.txt:3 (project):
  No CMAKE_CXX_COMPILER could be found.

  Tell CMake where to find the compiler by setting either the environment
  variable "CXX" or the CMake cache entry CMAKE_CXX_COMPILER to the full path
  to the compiler, or to the compiler name if it is in the PATH.

OK, no better way I know without using any bash condition variable.

Ex to build with clang.

```
export LLVM_DIR=/usr/src/clang-android
export LLVM_CONFIG=$LLVM_DIR/bin/llvm-config
COMPILE_FLAGS="-DCMAKE_CXX_COMPILER=${LLVM_DIR}/bin/clang++ -DLLVM_DIR=${LLVM_DIR}" source build.sh
```
JOBS=$(nproc) bash build.sh
@joeyjiaojg
Copy link
Contributor Author

@yuleisui ok, this should be final?

people can choose to build with clang and other JOBS value

export LLVM_DIR=/usr/src/clang-android && export LLVM_CONFIG=$LLVM_DIR/bin/llvm-config && JOBS=$(nproc) COMPILE_FLAGS="-DCMAKE_CXX_COMPILER=${LLVM_DIR}/bin/clang++ -DLLVM_DIR=${LLVM_DIR}" ./build.sh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants