This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Use unique_function in AbstractOperation fields
ClosedPublic

Authored by math-fehr on May 24 2021, 8:53 AM.

Details

Summary

Currently, AbstractOperation fields are function pointers.
Modifying them to unique_function allow them to contain
runtime information.

For instance, this allows operations to be defined at runtime.

Diff Detail

Event Timeline

math-fehr created this revision.May 24 2021, 8:53 AM
math-fehr requested review of this revision.May 24 2021, 8:53 AM
mehdi_amini accepted this revision.May 24 2021, 9:16 AM
This revision is now accepted and ready to land.May 24 2021, 9:16 AM
math-fehr updated this revision to Diff 347680.May 25 2021, 8:12 AM

Remove unneeded lambdas, and attempt to fix compilation on windows

math-fehr updated this revision to Diff 347695.May 25 2021, 8:48 AM

Try to fix windows compilation

math-fehr updated this revision to Diff 347706.May 25 2021, 9:26 AM

Try to fix windows compilation

Comment compilation problem on Windows

There seems to be a compilation problem on windows, and my last two patches fix it.

Basically, if A is a class and foo a templated static method of A, then constructing a unique_function with unique_function(&A::foo<Args>) triggers a compilation error C2298 on windows. I believe that this is a bug in the windows compiler, since this is a pointer to a static method.

I found a workaround using lambdas to make my code work on windows, but I believe that this can have a runtime cost.
I would be interested if anyone has a better solution!

rriddle added inline comments.May 25 2021, 10:33 AM
mlir/include/mlir/IR/OpDefinition.h
1684

Accidental change?

1708

Just remove these comments, there won't really be any runtime cost difference to using a lambda in a majority of cases because the function will just get inlined.

1778

Just use a lambda here.

mlir/lib/Parser/Parser.cpp
1806

Drop the llvm:: here.

rriddle accepted this revision.May 25 2021, 10:34 AM

Address comments

math-fehr marked 4 inline comments as done.May 25 2021, 11:34 AM

Thanks, I addressed the comments.

Also, I do not have commit access, so could one of you land it?

This revision was landed with ongoing or failed builds.May 25 2021, 11:40 AM
This revision was automatically updated to reflect the committed changes.