[PATCH] D18822: [Polly][FIX] Adjust execution context of hoisted loads wrt. error domains

[Johannes Doerfert via llvm-commits]

On 04/07, Tobias Grosser wrote:
> On 04/07/2016 03:36 PM, Johannes Doerfert wrote:
> >On 04/07, Tobias Grosser wrote:
> >>On 04/07/2016 01:32 PM, Johannes Doerfert wrote:
> >>>On 04/07, Tobias Grosser wrote:
> >>>>================
> >>>>Comment at: lib/Analysis/ScopInfo.cpp:3060
> >>>>@@ -3046,1 +3059,3 @@
> >>>>
> >>>>+isl_set *Scop::getErrorCtxReachingStmt(ScopStmt &Stmt) {
> >>>>+  auto *BB = Stmt.getEntryBlock();
> >>>>----------------
> >>>>Add __isl_give here as well.
> >>>We (or at least I) do not use isl memory annotations in the CPP file if
> >>>it is not a static function. Apparently you want me to now but not
> >>>before so I am confused.
> >>
> >>I may have missed this before. I would suggest to agree on something.
> >>
> >>$grep '^isl_' lib/*/*.cpp | wc
> >>      14      69    1151
> >>
> >>$grep '^__isl_' lib/*/*.cpp | wc
> >>      63     298    5010
> >>
> >>It seems we have both styles, with the second more common than the first. I
> >>personally prefer the second style, as it makes this information readily
> >>available when looking at the implementation of a function.
> >>
> >>Do you suggest to switch to your style instead?
> 
> >I don't know. I just noticed I (usually) do not use the __isl_* in the
> >cpp if it is not static, however that doesn't seem to be the case often.
> >If you prefer the second style we can use that one, but then I will
> >immediately raise the same question for arguments in the definition.
> >Do we want:
> >   ScopArrayInfo::getFromAccessFunction(__isl_keep isl_pw_multi_aff *PMA) {
> >     ...
> >   }
> >or
> >   ScopArrayInfo::getFromAccessFunction(isl_pw_multi_aff *PMA) {
> >     ...
> >   }
> 
> I would go for the first. The same reasoning applies and it is also more
> consistent.
Ok.

> If we agree on this, 
Good for me.

> I propose to add the annotations at places where they are missing.
> (Obviously, this is not coupled to this patch. Just a note for
> myself).
Let's do that at some point soon (in one commit) and from now on look
for it in patches.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 213 bytes
Desc: Digital signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160407/72c9c59d/attachment.sig>