[PATCH] D77787: [MLIR] Introduce a trait that defines a new scope for auto allocation

Uday Bondhugula via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 9 10:41:35 PDT 2020


bondhugula added a comment.

In D77787#1972255 <https://reviews.llvm.org/D77787#1972255>, @ftynse wrote:

> In D77787#1972235 <https://reviews.llvm.org/D77787#1972235>, @bondhugula wrote:
>
> > Just using a trait that doesn't use an OperationProperty bit is pretty straightforward, but simply landing the patch quickly isn't the point. What's more important here is to have the understanding and information documented on when something is a trait and an 'OperationProperty' as well - for the benefit of those adding traits in the future. This should go into the doc comment on 'OperationProperty'. I'm assuming you aren't blocked by this revision.
>
>
> We are in a situation where we have upstream code for IR that has unclear/unspecified behavior, which was pointed out by reviewers. I consider this to be an issue similar to a new bug being introduced by a change, i.e. we may want to revert the offending change. The present diff specifies this behavior, so I consider it a fix to a problem upstream. As such, it should land once accepted instead being blocked on a missing comment, that has been missing for more than a year.


There is still a blocking review on this - I don't think I can commit it yet.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77787/new/

https://reviews.llvm.org/D77787





More information about the llvm-commits mailing list