[Polly] Generating code for changed memory accesses

Johannes Doerfert doerfert at cs.uni-saarland.de
Sat Aug 2 18:57:20 PDT 2014


Patched in: r214657, r214658, r214659.

Some comments about the review are inlined.

On 08/02, Yabin Hu wrote:
> "ported" => "port"
Thx.
> typo, "then" => "than"
Thx.
> typo, "then" => "than"
Thx.

> TODO always ends with a dot, right?
Thx.

> Use alphabetical order:
> +class IslExprBuilder;
> +class MemoryAccess;
I kept the order as I'm not aware of any guideline we follow here.
If we start doing that I would like an official statement and a patch to change
at least a chunk of the existing code. If I start doing that in every patch I
either change the other lines and cause a lot of unrelated changes or I keep the
other lines and produce a mix of code which doesn't help us at all.

> -  /// @param Builder   The LLVM-IR Builder used to generate the statement.
> > The
> > -  ///                  code is generated at the location, the Builder
> > points to.
> > -  /// @param Stmt      The statement to code generate.
> > -  /// @param GlobalMap A map that defines for certain Values referenced
> > from the
> > -  ///                  original code new Values they should be replaced
> > with.
> > -  /// @param P         A reference to the pass this function is called
> > from.
> > -  ///                  The pass is needed to update other analysis.
> > +  /// @param Builder     An IR Builder to generate LLVM code.
> > +  /// @param Stmt        The statement to generate code for.
> > +  /// @param GlobalMap   A map from old values to new versions.
> > +  /// @param P           A pass to update other analysis.
> > +  /// @param Build       The AST build with the new schedule.
> > +  /// @param ExprBuilder An expression builder to generate new access
> > functions.
> 
> 
> I would like to just update the format rather than change the comments at
> the same time. I prefer the original comments.
That's fine with me (Idon't agree but don't feel strong about it in order to
oppose your opinion).

> > +#include "polly/CodeGen/IslExprBuilder.h"
> Using alphabetical order: put this include down after #include
> "polly/CodeGen/CodeGeneration.h"
Done.

> > +  Expr = isl_ast_build_access_from_pw_multi_aff(Build, PWAccRel);
> > +  return ExprBuilder->create(Expr);
> Add a blank line before return statement.
Done.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 213 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140802/77ba9de6/attachment.sig>


More information about the llvm-commits mailing list