Details
- Reviewers
efriedma jdoerfert Meinersbur gareevroman sebpop • zinob huihuiz pollydev grosser singam-sanjay - Commits
- rG35de9009171e: [NFC] Move PPCGCodeGeneration::pollyBuildAstExprForStmt to isl++.
rGf7face4bc435: Convert GPUNodeBuilder::getArraySize to islcpp.
rPLO308870: Convert GPUNodeBuilder::getArraySize to islcpp.
rPLO308869: [NFC] Move PPCGCodeGeneration::pollyBuildAstExprForStmt to isl++.
rL308870: Convert GPUNodeBuilder::getArraySize to islcpp.
rL308869: [NFC] Move PPCGCodeGeneration::pollyBuildAstExprForStmt to isl++.
Diff Detail
- Build Status
Buildable 8510 Build 8510: arc lint + arc unit
Event Timeline
Except of the minor issue, that looks fine to me.
lib/CodeGen/PPCGCodeGeneration.cpp | ||
---|---|---|
1021 | Use isl::manage(isl_multi_pw_aff_copy(Array->bound)); Otherwise, you take ownership of th the reference in Array->bound and you will free it unexpectedly in the future. | |
1033 | Drop this line. It is not needed after the change above. What you did here is to extract the object out of ArrayBound, but then ignore the return value and let it leak. This just works, because it matches up with the error above, |
lib/CodeGen/PPCGCodeGeneration.cpp | ||
---|---|---|
1033 | Yes, I did it this way to create a "temporary" C++ interface that I then release. However, I suppose copying and then allowing the destructor to manage memory is less error prone. |
I don't think the concept of a "temporary C++ interface" exists. To me it looks like too memory errors that hide each other. We we at some point write a static analyzer to check for memory errors, it will believe that the Array->bound object is invalid after your original code sequence.
The commit r308869 was incorrectly labeled - it was supposed to close D35770. Writing this here as documentation.
@grosser I do not believe so, release sets the ptr within the isl::multi_pw_aff to nullptr. So, the destructor will not free anything.
__isl_give isl_multi_pw_aff *multi_pw_aff::release() { isl_multi_pw_aff *tmp = ptr; ptr = nullptr; return tmp; }
It shouldn't leak memory, it just looks awkward.
Use isl::manage(isl_multi_pw_aff_copy(Array->bound));
Otherwise, you take ownership of th the reference in Array->bound and you will free it unexpectedly in the future.