[Polly] Generating code for changed memory accesses

[Johannes Doerfert]

If I do 1) I will fail the tests, that's the reason I didn't split them.
I think 2) is a nice way to go, but I first need to move the privatization stuff to the IslAst level before I can use it the way you describe it.
Till then I'd think this way is more general as it allows both use cases (early or late creation of the isl_ast_exprs).

Ok, what's the final decision on how we proceed?

--

Johannes Doerfert
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


-----Original Message-----
From: Tobias Grosser [mailto:tobias at grosser.es] 
Sent: Thursday, July 31, 2014 1:13 PM
To: Johannes Doerfert
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [Polly] Generating code for changed memory accesses

On 31/07/2014 08:13, Johannes Doerfert wrote:
> Attached the newest version which looks good I think.
>
> Thoughts?

Looks good so far.

Two remarks:

1)

As suggested in my earlier mail, I would prefer to move the tests first and then change the implementation. This makes it clear to the reader that the implementation does not change the tests, or, if it does, it makes clear which adjustments are necessary.
On the other side, there is no need to split of the IslExprBuilder changes as they are not tested at all in the separate patch.


2)

I also wonder if we actually want to compute the isl_ast_expr's already in IslAst.cpp. I think about this for two reasons:

  a) this would allow us to printing accesses and write test cases that
     check for those pretty printed changed accesses

  b) I think Yabin will not modify the access relations, but instead
     directly provide isl_ast_exprs. (In fact he is not even using the
     IslAst.cpp pass, but computes an entirely new ast). So from his
     perspective the IslCodeGeneration class is really an utility class
     that translates a certain IslAst into the corresponding LLVM-IR.

I would like you to address 1). Regarding 2): I think it would be nice to already do this in this patch, but if you prefer to get it in as it is and have a later discussion in case someone starts to work on dumping accesses in -polly-ast or in case Yabin starts to make use of this, that is fine with me as well.

Cheers,
Tobias