- User Since
- Sep 16 2016, 11:47 AM (213 w, 3 d)
Fri, Oct 2
Hey! Thanks for the patch! I wondered if it would make sense to move this utility to a public utility header. I found myself needing it locally and I wouldn't like to just copy-paste it. There is another topological sort for ops under 'include/mlir/Analysis/SliceAnalysis.h'. Maybe we could move them both to 'include/mlir/IR/Utils.h'? WDYT?
Thu, Oct 1
Tue, Sep 29
Thanks, Alex. Will do.
Mon, Sep 28
Sat, Sep 26
Thanks, LGTM! Please, give some time for @ftynse to have a look.
Thanks, Nicolas. The failure doesn't seem related to this:
Fri, Sep 25
Thu, Sep 24
Sorry, I misunderstood what an unranked memref models. This indeed doesn't make sense as is. Thanks.
- Removed support for unranked memrefs
- Addressed feedback
Thanks, Alex! Sorry, I misunderstood what an unranked memref is modeling. Let me remove that from the patch since it's incorrect and only leave the support for call and return ops. We can add support for unranked memrefs when needed.
Wed, Sep 23
Mon, Sep 21
As a general observation, do you folks actually want the bare-pointer convention, or is it only there to attach noalias argument attribute? Maybe we should consider investing into expressing aliasing information in MLIR rather than keep the maintenance burden.
- Refactor code to a utility function.
- Replace i64 type with index type.
- Address Alex's feedback.
Sep 15 2020
Sorry for the delay and sorry, Nicolas, I had written the example that
you were asking for in the .cpp doc but I forgot to copy it to the header
file. This patch is doing that. Please, let me know if there are any other
Sep 14 2020
This commit seems to be breaking the build with -DBUILD_SHARED_LIBS=TRUE. Could you please have a quick look? Thanks!
Sep 10 2020
LGTM! Just some nit comments! Thanks for contributing this feature!
Sep 4 2020
Sep 2 2020
Thanks! Any other comments, @aartbik?
Thanks! Any other comments? @aartbik?
Addressed feedback. Thanks!
Addressed feedback. Thanks!
Sep 1 2020
Thanks for the patch! I can confirm that it is fixing our build locally (using gcc 7.5.0).
Aug 31 2020
Thanks! Addressing feedback.
Addressed all the comments and replaced the DFS loop container in the API with
an 'std::vector<SmallVector<AffineForOp, 2>>' (we use something similar in other
loop utilities). This container is a bit more friendly for a public API than
providing a plain list of loops in DFS order and also explicitly represents the
nesting level relationship of the loops that we want to vectorized, which is
useful for the vectorization algorithm itself. The vectorizer is therefore updated
to work on this container.
Aug 27 2020
Thanks, Aart! Sorry for the delay. I've been OOO. I'll take a look at your comments and I'll also try to use 'std::vector<SmallVector<AffineForOp, 2>>' for the API so that we can preserve some kind of loop nest structure.
Aug 24 2020
Thanks Frank! Sorry, I was OOO last week. Quick comments for now, I'll come back later.
Aug 16 2020
Unblocking the review from my side. OOO next week.
Thanks for addressing the comments, Frank! LGTM. I'll leave the final approval to Uday and River since I'm OOO next week.
Aug 12 2020
Aug 10 2020
Aug 6 2020
Aug 5 2020
Thanks Frank! It looks good to me. Just a few minor comments
Aug 4 2020
Aug 3 2020
Thanks for the patch! Just a high-level comment for now. It looks good overall.
Jul 28 2020
Jul 27 2020
Jul 23 2020
Jul 22 2020
Jul 21 2020
Jul 20 2020
Jun 26 2020
Thanks Jeremy! Some minor comments.
Jun 25 2020
Jun 24 2020
Jun 23 2020
Thanks for fixing this! LGTM
Jun 15 2020
Jun 11 2020
Jun 10 2020
Jun 8 2020
Thanks Uday! Good point. I haven't looked too much into the slice computation but if you can send me some pointers and some examples on how the regions should be computed I could have a look. Thanks!
Jun 5 2020
If no more comments, I'll land this after https://reviews.llvm.org/D81302