[mlir][WIP] symbol_source attr in linalg.generic proposal

[limo1996]

Summary

Depends On D83191

[limo1996]

Linting message fixed

[ftynse]

mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td

521

I think llvm::Optional<int> is preferable to returning implicit -1 as a sign of the attribute being absent.

mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp

321 ↗

(On Diff #276367)

Do you see a good reason why this code (and below) is using the default autogenerated builder form instead of the custom builder form? Maybe it is worth updating so that next modifications of the Op arguments don't require changes.

mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp

529

We shouldn't be having this attribute on the IndexedGenericOp.

mlir/test/Dialect/Linalg/invalid.mlir

124 ↗

(On Diff #276367)

There's no need to check autogenerated error messages

[limo1996]

mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td

521

Ahh Thank you. Didn't know about this option.

mlir/test/Dialect/Linalg/invalid.mlir

124 ↗

(On Diff #276367)

Good to know. Should I remove the check completely?

[limo1996]

mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp

529

Yeah that's kind of the point of this revision.. I know we should not but I wanted to ask how can I solve it bcos I don't know how to add to arguments instead of rewrite them inside GenericOp definition.. Any suggestion is rly welcomed :)

[ftynse]

mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td

521

Just change the return type and return llvm::None instead of -1.

mlir/test/Dialect/Linalg/invalid.mlir

124 ↗

(On Diff #276367)

Yes

[limo1996]

getSymbolSource: llvm::Optional instead of -1

[limo1996]

mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp

321 ↗

(On Diff #276367)

So the problem is that custom builder requires types like int64_t etc. but autogenerated one requires corresponding attributes.. So I would suggest one more custom builder but with attributes. What do you think?

[limo1996]

Test cases for now autogenerated error messages removed

[limo1996]

mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp

321 ↗

(On Diff #276367)

Submited it as a bug. You can find it here: https://bugs.llvm.org/show_bug.cgi?id=46658

[limo1996]

changes evolved from revision D83158

[limo1996]

evolve

[limo1996]

Changes merged to D83158

mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td

let extraClassDeclaration = [{
     llvm::Optional<SmallVector<StringRef, 8>> referenceIterators() {
       llvm_unreachable(
         "No such thing as reference iterator types for a generic op.");
     }
 
     llvm::Optional<SmallVector<AffineMap, 8>> referenceIndexingMaps() {
       llvm_unreachable(
         "No such thing as reference indexing maps for a generic op.");
      }
-
-    llvm::Optional<int> getSymbolSource() {
-      auto ss = symbol_source();
-      return ss.hasValue() ?
-        llvm::Optional<int>(ss.getValue().getLimitedValue()) : llvm::None;
-    }
   }];
 
   let printer = [{ return ::print(p, *this); }];
   let parser = [{ return ::parseGenericOp(parser, result); }];
 }
 
 /// Index-free GenericOp.
 def GenericOp : GenericOpBase<"generic"> {
   let description = [{
     Generic Linalg op form where the key properties of the computation are
     specified as attributes. In pretty form, a linalg.generic op is written as:

mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp

if (isa<GenericOp>(producer.getOperation()) &&
                   consumer.getOperation()->getResultTypes(), fusedOperands,
                   rewriter.getI64IntegerAttr(fusedOperands.size()),
                   rewriter.getI64IntegerAttr(
                       consumer.getOperation()->getNumResults()),
                   rewriter.getArrayAttr(fusedIndexMaps),
                   consumer.iterator_types(),
                   /*doc=*/nullptr,
                   /*library_call=*/nullptr,
-                  /*symbol_source=*/nullptr)
+                  /*symbol_source*/ nullptr)
               .getOperation();
     }
 
     // Construct an AffineMap from consumer loops to producer loops.
     // consumer loop -> tensor index
     AffineMap consumerResultIndexMap =
         consumer.getInputIndexingMap(consumerIdx);
     // producer loop -> tensor index