This is an archive of the discontinued LLVM Phabricator instance.

[MLIR][LinAlg] Fix a small compilation error.
AbandonedPublic

Authored by ergawy on May 7 2021, 12:32 AM.

Details

Diff Detail

Event Timeline

ergawy created this revision.May 7 2021, 12:32 AM
ergawy requested review of this revision.May 7 2021, 12:32 AM

I might have misunderstood something. Let me know if I should abandon this in case this a local issue on my machine or there is a better solution.

gysit added a comment.May 7 2021, 12:40 AM

Thanks for the fix!

I believe the builder takes an int64_t:

void ConstantIndexOp::build(OpBuilder &builder, OperationState &result,
                            int64_t value) {

Would casting to int64_t work as well? And out of curiosity for which compiler do you see the issue?

ergawy added a comment.EditedMay 7 2021, 12:46 AM

I use: Apple clang version 11.0.3.

What made me think it is a local issue (after submitting the patch), is that there are 2 builders generated in LinalgOps.h.inc (for IndexOp which is used in this line not ConstantIndexOp):

static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, int64_t dim);
static void build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, uint64_t dim);

Maybe it's a case of needing a clean build on my end? Or is this on purpose?

gysit accepted this revision.May 7 2021, 12:51 AM

well I added an additional builder that takes an int64_t dimension argument. I guess the proper solution is to remove one of the builders. I am still a bit confused you get a compilation error. Usually the compiler should chose the overload that is closest?

The clean solution is probably to remove one of the two builders. I think you code is also fine and I am fine with pushing it if it helps. Note that the pattern is temporary code anyways. Maybe try a rebuild and push if this doesn't help. I will investigate if I can remove the builder in meantime.

This revision is now accepted and ready to land.May 7 2021, 12:51 AM
ergawy abandoned this revision.May 7 2021, 7:01 AM

Abandoned in favor of: D102055.