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

Add env var to save generated kernels src to tmp dir #2404

Merged
merged 8 commits into from
Jan 12, 2019

Conversation

WilliamTambellini
Copy link
Contributor

Add an env var option to save generated JIT kernels src to a folder.
#2310

Copy link
Member

@umar456 umar456 left a comment

Choose a reason for hiding this comment

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

This looks good. I want to make the environment variable interface slightly more flexible for future changes. Also could you add documentation for this variable in https://github.com/arrayfire/arrayfire/blob/master/docs/pages/configuring_arrayfire_environment.md?

@@ -15,3 +15,7 @@
#pragma once

std::string getEnvVar(const std::string &key);

// Dump the kernel sources only if the environment variable is defined
static const char* saveJitKernelsEnvVarName = "AF_DUMP_JIT_KERNELS_DIR";
Copy link
Member

Choose a reason for hiding this comment

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

Can you rename this to AF_JIT_KERNEL_TRACE?

It accepts 1 and writes it to stdout or "directory:..." and it will create a file for each kernel. In the future I will be creating a graphviz images so I want this to be slightly more flexible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed. Now accepts stdout/stderr.

@@ -15,3 +15,7 @@
#pragma once

std::string getEnvVar(const std::string &key);

// Dump the kernel sources only if the environment variable is defined
static const char* saveJitKernelsEnvVarName = "AF_DUMP_JIT_KERNELS_DIR";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to let you officially publish that option in the doc when the specs will be totally stable.

@WilliamTambellini
Copy link
Contributor Author

Tks Umar. @pavanky and others : please review : it s today not possible to inspect/look at jit kernel sources without that change.

@umar456
Copy link
Member

umar456 commented Jan 8, 2019

Only one review is required. The branch has a couple of conflicts that I am going to address in a bit.

@umar456 umar456 merged commit 4bdaa9e into arrayfire:master Jan 12, 2019
9prady9 pushed a commit to 9prady9/arrayfire that referenced this pull request Apr 8, 2019
* Use AF_JIT_KERNEL_TRACE environment variable to save kernel to file or display on stdout
  or stderr

(cherry picked from commit 4bdaa9e)
umar456 pushed a commit to 9prady9/arrayfire that referenced this pull request Apr 17, 2019
* Use AF_JIT_KERNEL_TRACE environment variable to save kernel to file or display on stdout
  or stderr

(cherry picked from commit 4bdaa9e)
umar456 pushed a commit that referenced this pull request Apr 17, 2019
* Use AF_JIT_KERNEL_TRACE environment variable to save kernel to file or display on stdout
  or stderr

(cherry picked from commit 4bdaa9e)
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