Skip to content

tiny improvement - #1093

Open
joeyjiaojg wants to merge 2 commits into
SVF-tools:masterfrom
joeyjiaojg:master
Open

tiny improvement#1093
joeyjiaojg wants to merge 2 commits into
SVF-tools:masterfrom
joeyjiaojg:master

Conversation

@joeyjiaojg

@joeyjiaojg joeyjiaojg commented May 16, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor
  1. pass JOBS to build.sh
  2. pass COMPILE_FLAGS to cmake config

yuleisui
yuleisui reviewed May 16, 2023
View reviewed changes
Comment thread build.sh Outdated
Comment thread build.sh Outdated
Comment thread svf/include/Util/Annotator.h
joeyjiaojg
joeyjiaojg commented May 16, 2023
View reviewed changes

@joeyjiaojg joeyjiaojg left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated, please check again?

Comment thread build.sh Outdated
Comment thread build.sh Outdated
@joeyjiaojg
joeyjiaojg force-pushed the master branch 3 times, most recently from 10fdd04 to 7660ea6 Compare May 16, 2023 06:01
joeyjiaojg
joeyjiaojg commented May 16, 2023
View reviewed changes

@joeyjiaojg joeyjiaojg left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done, pls review again.

yuleisui
yuleisui reviewed May 16, 2023
View reviewed changes
Comment thread build.sh Outdated
set -e # exit on first error

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

@yuleisui yuleisui May 16, 2023

Copy link
Copy Markdown
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

@yuleisui yuleisui May 16, 2023

Copy link
Copy Markdown
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

@joeyjiaojg joeyjiaojg May 18, 2023

Copy link
Copy Markdown
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.

Comment thread build.sh Outdated
Comment thread build.sh
@codecov

codecov Bot commented May 16, 2023 •
edited
Loading

Copy link
Copy Markdown

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

yuleisui
yuleisui reviewed May 16, 2023
View reviewed changes
Comment thread build.sh Outdated
@joeyjiaojg
joeyjiaojg force-pushed the master branch 2 times, most recently from 46333d1 to 23b8c01 Compare May 18, 2023 06:53
@joeyjiaojg

joeyjiaojg commented May 18, 2023

Copy link
Copy Markdown
Contributor Author

The new patch should be better now.

@joeyjiaojg

joeyjiaojg commented May 18, 2023

Copy link
Copy Markdown
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.

joeyjiaojg added 2 commits May 18, 2023 15:34
@joeyjiaojg Security CI
add COMPILE_FLAGS pass to cmake config
d8108a2 
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
```
@joeyjiaojg Security CI
JOBS to override default jobs
3a2f707 
JOBS=$(nproc) bash build.sh
@joeyjiaojg

joeyjiaojg commented May 18, 2023

Copy link
Copy Markdown
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

Reviewers

@yuleisui yuleisui yuleisui left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@joeyjiaojg @yuleisui