This is an archive of the discontinued LLVM Phabricator instance.

[MLIR] Bug fix for simplifyAffineMap
AbandonedPublic

Authored by jbruestle on Apr 14 2020, 1:04 PM.

Download Raw Diff

Details

Reviewers

bondhugula
rriddle
nicolasvasilache
flaub

Summary

When simplifying a 0-dimensional affine map (i.e. when map.getNumResults() == 0), AffineMap::get needs to have access to a valid context.

Diff Detail

Event Timeline

jbruestle created this revision.Apr 14 2020, 1:04 PM

Herald added 1 blocking reviewer(s): rriddle. · View Herald TranscriptApr 14 2020, 1:04 PM

Herald added a project: Restricted Project. · View Herald Transcript

Herald added subscribers: llvm-commits, frgossen, grosul1 and 10 others. · View Herald Transcript

LGTM

rriddle requested changes to this revision.Apr 14 2020, 1:36 PM

rriddle added inline comments.

mlir/lib/IR/AffineMap.cpp
326	This diff looks backwards

This revision now requires changes to proceed.Apr 14 2020, 1:36 PM

bondhugula added inline comments.Apr 14 2020, 1:39 PM

mlir/lib/IR/AffineMap.cpp
325	Is there an issue with the way this revision was created? The left/right panes are switched for me (the right one has the old version), and the context isn't available as well.

Could you add a test case please? It could be added to test/Dialect/Affine/simplify-affine-structures.mlir (the simplification is called on the map attribute of any op there).

Harbormaster failed remote builds in B53224: Diff 257458!Apr 14 2020, 2:05 PM

Preparing more more comprehensive diff as per discussion in the mailing list.

Revision Contents

Path

Size

mlir/

lib/

IR/

AffineMap.cpp

2 lines

Diff 257458

mlir/lib/IR/AffineMap.cpp

Context not available.
	exprs.push_back(	exprs.push_back(
	simplifyAffineExpr(e, map.getNumDims(), map.getNumSymbols()));	simplifyAffineExpr(e, map.getNumDims(), map.getNumSymbols()));
	}	}
	return AffineMap::get(map.getNumDims(), map.getNumSymbols(), exprs, map.getContext());	return AffineMap::get(map.getNumDims(), map.getNumSymbols(), exprs);
		bondhugulaUnsubmitted Not Done Reply Inline Actions Is there an issue with the way this revision was created? The left/right panes are switched for me (the right one has the old version), and the context isn't available as well. bondhugula: Is there an issue with the way this revision was created? The left/right panes are switched for…
	}	}
		rriddleUnsubmitted Not Done Reply Inline Actions This diff looks backwards rriddle: This diff looks backwards

	AffineMap mlir::removeDuplicateExprs(AffineMap map) {	AffineMap mlir::removeDuplicateExprs(AffineMap map) {
Context not available.