Skip to content

Conversation

@pigrew
Copy link
Contributor

@pigrew pigrew commented May 3, 2018

This adds a new environment variable, PTXPLUS_SIM_KERNELFILE , which can be used to override the PTXPLUS file used in the simulation. This is useful, for example, when doing register reallocation. Currently the code still uses the ptxinfo from the original PTX output to determine the number of registers, etc, that are needed. It doesn't seem like there are any functions to determine the register count from the ptxplus (and ptxas can't read ptxplus).

This fixes some bugs with respect to using "delete" to free memory which was malloced. It also fixes some memory leaks of PTX.

@tgrogers tgrogers self-requested a review October 5, 2018 16:31
//! Read file into char*
//TODO: convert this to C++ streams, will be way cleaner
char* readfile (const std::string filename){
std::unique_ptr<char[]> readfile (const std::string filename){
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the motivation for removing the char*?

Copy link
Contributor Author

@pigrew pigrew Oct 9, 2018

Choose a reason for hiding this comment

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

There was a memory leak in the calling code. The choices were to either use the auto-freeing unique_ptr or to add a free to the calling code. It's a coding convention question if it's desired that the code should should malloc/free or if it should use the "modern" C++ features.

Also it looks like I had commented that there was some "malloc" mixed with "delete" or "new" mixed with "free. This is against the C++ standard, but probably works with most standard libraries.

ptxcode = readfile(override_ptx_name);
}
if(context->get_device()->get_gpgpu()->get_config().convert_to_ptxplus() ) {
const char *override_ptxplus_name = getenv("PTXPLUS_SIM_KERNELFILE");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure we need another variable for this - we should probably just use the PTX_SIM_KERNELFILE variable and not override even if "context->get_device()->get_gpgpu()->get_config().convert_to_ptxplus()". Basically just change the "if(context->get_device()->get_gpgpu()->get_config().convert_to_ptxplus() ) {" at line 1729 to "else if"

aamodt pushed a commit that referenced this pull request Feb 11, 2025
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