[PATCH] D76479: [mlir][LLVMIR] Fix fusion for rank-0 tensors
Mahesh Ravishankar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 20 10:50:03 PDT 2020
mravishankar added inline comments.
================
Comment at: mlir/lib/IR/AffineMap.cpp:260
- return get(numResultDims, numResultSyms, results);
+ return results.empty() ? get(numResultDims, 0, this->getContext())
+ : get(numResultDims, numResultSyms, results);
----------------
asaadaldien wrote:
> mravishankar wrote:
> > There are going to be so many of such cases. The get method that gets context from the result expression should just be removed IMO
> IMO we should have a single ::get method that handle any results exprs including empty e.g
> ```
> AffineMap AffineMap::get(unsigned dimCount, unsigned symbolCount,
> ArrayRef<AffineExpr> results) {
>
> if (!results.empty()) {
> getImpl(dimCount, symbolCount, results, results[0].getContext());
> } else {
> assert(symbolCount == 0);
> return get(numResultDims, 0, this->getContext());
> }
> }
> ```
> Not sure if Nicolas prefer to have an explicit get method for rank-0 tensors for a specific reason.
There is no `this->getContext()` . This is a static class method. So this would not compile.
I think the reason for the separate method is because of the current `assert(!result.empty())`. Should just have a single method
```
AffineMap AffineMap::get(unsigned dimCount, unsigned symbolCount, ArrayRef<AffineExpr> results, MLIRContext *context) {
return getImpl(dimCount, symbolCount, results, context);
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76479/new/
https://reviews.llvm.org/D76479
More information about the llvm-commits
mailing list