[SYCL] Fix memory leak in online compiler #4963
Conversation
The experimental online compiler may leak memory in compileToSPIRV. These changes address this leak by storing the SPIR-V binary information directly in the vector that will later be returned. Signed-off-by: Steffen Larsen <[email protected]>
| SpirVSize = OutputLengths[I]; | ||
| SpirV = new byte[SpirVSize]; | ||
| std::memcpy(SpirV, Outputs[I], SpirVSize); |
There was a problem hiding this comment.
@v-klochkov - Can there only be at most one .spv in OutputNames?
There was a problem hiding this comment.
@v-klochkov, ping, can you answer the question above?
There was a problem hiding this comment.
The changes look good to me.
Regarding the number of output spirv files. It is assumed to be 1 at most, but I did not find any strict wordings about that in ocloc API https://github.com/intel/compute-runtime/blob/master/shared/offline_compiler/source/ocloc_api.h
It may be a good idea to add an assert here checking that there is only 1 spv output.
There was a problem hiding this comment.
Good idea! I have added such an assertion.
| SpirVSize = OutputLengths[I]; | ||
| SpirV = new byte[SpirVSize]; | ||
| std::memcpy(SpirV, Outputs[I], SpirVSize); |
There was a problem hiding this comment.
The changes look good to me.
Regarding the number of output spirv files. It is assumed to be 1 at most, but I did not find any strict wordings about that in ocloc API https://github.com/intel/compute-runtime/blob/master/shared/offline_compiler/source/ocloc_api.h
It may be a good idea to add an assert here checking that there is only 1 spv output.
Signed-off-by: Steffen Larsen <[email protected]>
steffenlarsen
dismissed stale reviews from v-klochkov and alexbatashev
via
43e98f7
The experimental online compiler may leak memory in compileToSPIRV. These changes address this leak by storing the SPIR-V binary information directly in the vector that will later be returned.