LICM pass zero trip count loop handling; zero trip count loop removal in

[bondhugula]

-simplify-affine-structures

• addresses Consider trip count in loop invariant code motion #194

• change name of tablegen auto-generated internal helper for op
  interfaces to avoid potential conflicts with methods of same
  name in dialect namespaces. (addresses Operation interfaces: can't name the interface method same as free function on op #197)

Signed-off-by: Uday Bondhugula [email protected]

[joker-eph]

This is a problem in terms of dependencies (Bazel is catching some cycles for us here).

The problem is that while the LoopOps.h which defines the LoopOps dialect is correctly providing a self-contained version of getConstantTripCount, the Affine dialect is not: instead it relies on the getConstantTripCount provided by the Analysis library.
Having the affine dialect itself be dependent on the Analysis library, while the Analysis library is also depending on Affine does not seem right.

I suspect all the Affine specific Analysis should be moved into the affine dialect to avoid the cycle.

[bondhugula]

This issue was discussed on this thread: https://groups.google.com/a/tensorflow.org/forum/?utm_medium=email&utm_source=footer#!msg/mlir/6DV_IbyfnQ8/0Ub44e0eDgAJ

While moving the Analysis into the dialect is one solution, the plan from the start has been to keep ops separate from analysis and transforms, and not have a dependence from the dialect ops to analysis/transforms. The issue here is that LoopInterface.h is needed both for "op support" and "analysis support". In this scenario which is a first I think, there is a free function being generated via the operation interface. If we had a separate header for it, we wouldn't have that being included from the affine dialect. Note that the affine dialect is not using getConstantTripCount anywhere inside (only those that already depend on Analysis use it). It's actually odd / out of line that something from "mlir/Transforms/" is being included into include/mlir/Dialect/AffineOps/AffineOps.h! (see here:

mlir/include/mlir/Dialect/AffineOps/AffineOps.h

Line 31 in 2bd4e07

#include "mlir/Transforms/LoopLikeInterface.h"

)

So, the larger issue is that: since operation interfaces could be used to generate reusable op get/set methods as well as analysis/transform methods, shouldn't we be segregating their headers? If we want to go with merging affine analysis and ops, at some point, the issue of merging standardops and analysis/transforms is also likely to come up.

[joker-eph]

I think in general we'll have something like:

• lib/Transforms & lib/Analysis : should contain core logic, using only interfaces. These should not depend on dialects like Affine.
• lib/Dialect/XYZ/Transforms & lib/Dialect/XYZ/Analysis : dialect specific passes. For instance we have already lib/Dialect/Linalg/Analysis/ and lib/Dialect/Linalg/Transforms/
• include/mlir/Interfaces/... : self-contained interface headers intended to implemented/overridden in dialects and used everywhere.

(I haven't thought much further, there might be another reasonable and scalable option)

[bondhugula]

Yes, but this doesn't solve the dependence cycle issue we have here unless you have:

include/mlir/Interfaces/Ops/...
include/mlir/Interfaces/Analysis/
include/mlir/Interfaces/Transforms/

(Because the cycle in context is created due to dialect ops depending on Interfaces and Interfaces depending on Analysis. Having the above separation would mean a Dialect/XYZ/Ops would only depend on InterfaceOps and InterfaceAnalysis can depend on Analysis/ or any Dialect/XYZ/Analysis. )

[joker-eph]

the cycle in context is created due to dialect ops depending on Interfaces and Interfaces depending on Analysis

Can you clarify how is this interface depending on Analysis?

I don't see a direct dependency other than the Affine situation. For instance the use of this interface in the loop dialect does not pull anything from Analysis as far as I can tell.

[bondhugula]

the cycle in context is created due to dialect ops depending on Interfaces and Interfaces depending on Analysis

Can you clarify how is this interface depending on Analysis?

The interface method getConstantTripCount depends on the analysis method mlir::getConstantTripCount for AffineForOp, but for loop::ForOp depends on ForOp::getConstantTripCount, which is simpler (doesn't need analysis) and is thus an op method.

I don't see a direct dependency other than the Affine situation. For instance the use of this interface in the loop dialect does not pull anything from Analysis as far as I can tell.

That's because: for loop::forOp, mlir::loop::getConstantTripCount doesn't rely on Analysis and lives on loop::ForOp itself. It's a method of loop::ForOp defined in Dialect/LoopOps/LoopOps.cpp unlike the free function mlir::getConstantTripCount(AffineForOp) in mlir/Analysis/LoopAnalysis.cpp.

[joker-eph]

I agree with what you wrote, but my conclusion is that the interface itself does not depends on Analysis. Only the affine dialect does.