Page MenuHomePhabricator

[mlir] Make ValueShapeRange a new class
ClosedPublic

Authored by jpienaar on Jul 25 2021, 1:33 PM.

Details

Summary

Retaining old interface and should be constructable as previous, change would have been NFC except it this doesn't implicitly work with OpAdaptor constructor in C++14. Beyond that no new information/accessors in class - follow up to enable passing in shapes compatible with values during refinement without mutating the IR.

Diff Detail

Event Timeline

jpienaar created this revision.Jul 25 2021, 1:33 PM
jpienaar requested review of this revision.Jul 25 2021, 1:33 PM

Can you provide the "why"? I'm not sure what the motivation is here.

mlir/include/mlir/Interfaces/InferTypeOpInterface.h
81–102

Why not just inherit from ValueRange?

jpienaar edited the summary of this revision. (Show Details)Jul 25 2021, 1:55 PM

Can you provide the "why"? I'm not sure what the motivation is here.

Sure, expanded description. This is basically towards avoiding to mutate IR during refinement. And will also be used to allow querying constant values without materializing them.

mlir/include/mlir/Interfaces/InferTypeOpInterface.h
81–102

ValueRange is final and I need to add members and accessors. I could also make it not be final, but assumed there was a reason & it is widely used so didn't want to impede optimizations.

rsuderman accepted this revision.Jul 26 2021, 9:31 AM

Looks good to me assuming @rriddle is satisfied.

This revision is now accepted and ready to land.Jul 26 2021, 9:31 AM
This revision was automatically updated to reflect the committed changes.
rriddle added inline comments.Jul 26 2021, 5:10 PM
mlir/include/mlir/Interfaces/InferTypeOpInterface.h
81

You won't be able to use RangeBaseT here. It is CRTP (on ValueRange) and there are several static_casts that rely on the derived type.