[PATCH] D33688: [Polly] Heap allocation for new arrays

[Andreas Simbuerger via Phabricator via llvm-commits]

simbuerg added inline comments.


================
Comment at: lib/Exchange/JSONExporter.cpp:722
+        auto CurrentSAI =
+            S.getArrayInfoByName(Arrays[ArrayIdx]["name"].asCString());
+        CurrentSAI->setIsOnHeap(true);
----------------
niosega wrote:
> simbuerg wrote:
> > niosega wrote:
> > > I need to have a pointer to the newly created ScopArrayInfo to call the setter.
> > > But the method createScopArrayInfo returns only a const SAI *. The solution 
> > > I found is to query the Scop to obtain the SAI by name.
> > > 
> > > An alternative would be to pass a parameter to createScopArrayInfo then to 
> > > getOrCreateScopArrayInfo that represents the value that isOnHeap must take.
> > This is just to circumvent an inconvenient API that you want to add with this patch. I would suggest a simpler way:
> > Instead of depending on the setter, just add the IsOnHeap property as a function argument to
> > createScopArrayInfo and pass it through to getOrCreateScopArrayInfo. There you can pass it to the constructor or use
> > your setter.
> To pass the parameter through createScopArrayInfo then getOrCreateScopArrayInfo  then to the constructor is what I did in the previous version of the patch. Michael and you agreed that it was not the prettiest solution. Should I change it back ?
Well the pass-through is ugly too :-).
So far, I see 3 options:

1) Pass it through everything.
2) Remove the const-ness of createScopArrayInfo
3) Make IsOnHeap mutable.

I just dislike the lookup by name to get a non-const ScopArrayInfo. Michael what do you think?
If nobody is objecting the name-lookup, I'm fine with it as well.


https://reviews.llvm.org/D33688