Page MenuHomePhabricator

Remove use of tuple for multi-result operation result type storage
ClosedPublic

Authored by jpienaar on Feb 28 2021, 6:23 PM.

Details

Summary

Move the result types to trailing type op storage instead. This
results in each operation having its own types recorded vs deduped tuple type,
but comes at benefit that every mutation doesn't incurs uniquing. The latter
ran into cases whereupdating result type of operation led to very large memory
usage.

Diff Detail

Event Timeline

jpienaar created this revision.Feb 28 2021, 6:23 PM
jpienaar requested review of this revision.Feb 28 2021, 6:23 PM
mehdi_amini added inline comments.Feb 28 2021, 8:13 PM
mlir/lib/IR/Operation.cpp
555–556

I don't think this is legal: you can only access the element of a union through the member it has been set IIRC.

jpienaar updated this revision to Diff 327085.Mar 1 2021, 5:30 AM
jpienaar marked an inline comment as done.

Only access union element that was set for case

I know River had a better/alternate approach in mind here too

mlir/lib/IR/Operation.cpp
555–556

Good, point, had kept it to close to the original logic and didn't think about that. Updated here and below.

mehdi_amini accepted this revision.Mar 1 2021, 9:15 AM
This revision is now accepted and ready to land.Mar 1 2021, 9:15 AM
This revision was landed with ongoing or failed builds.Mar 1 2021, 9:30 AM
This revision was automatically updated to reflect the committed changes.

It doesn't look like MSVC likes this use of type. Break in the buildbot here: https://lab.llvm.org/buildbot/#/builders/13/builds/5148. Thanks!

FAILED: tools/mlir/lib/IR/CMakeFiles/obj.MLIRIR.dir/Operation.cpp.obj 
...
C:\buildbot\mlir-x64-windows-ninja\llvm-project\mlir\lib\IR\Operation.cpp(174): error C2280: 'mlir::Operation::<unnamed-type-resultTypeOrSize>::<unnamed-type-resultTypeOrSize>(void)': attempting to reference a deleted function
C:\buildbot\mlir-x64-windows-ninja\llvm-project\mlir\include\mlir/IR/Operation.h(709): note: compiler has generated 'mlir::Operation::<unnamed-type-resultTypeOrSize>::<unnamed-type-resultTypeOrSize>' here
C:\buildbot\mlir-x64-windows-ninja\llvm-project\mlir\include\mlir/IR/Operation.h(709): note: 'mlir::Operation::<unnamed-type-resultTypeOrSize>::<unnamed-type-resultTypeOrSize>(void)': function was implicitly deleted because 'mlir::Operation::<unnamed-type-resultTypeOrSize>' has a variant data member 'mlir::Operation::<unnamed-type-resultTypeOrSize>::type' with a non-trivial default constructor
C:\buildbot\mlir-x64-windows-ninja\llvm-project\mlir\include\mlir/IR/Operation.h(706): note: see declaration of 'mlir::Operation::<unnamed-type-resultTypeOrSize>::type'

Thanks, reverted,