This is an archive of the discontinued LLVM Phabricator instance.

[MLIR] Add argument related API to Region
ClosedPublic

Authored by jurahul on Jul 10 2020, 5:08 PM.

Details

Summary
  • Arguments of the first block of a region are considered region arguments.
  • Add API on Region class to deal with these arguments directly instead of using the front() block.
  • Changed several instances of existing code that can use this API
  • Fixes https://bugs.llvm.org/show_bug.cgi?id=46535

Diff Detail

Event Timeline

jurahul created this revision.Jul 10 2020, 5:08 PM
bondhugula accepted this revision.Jul 12 2020, 8:55 AM
bondhugula added a subscriber: bondhugula.

Please check the build failures though.

This revision is now accepted and ready to land.Jul 12 2020, 8:55 AM
mehdi_amini requested changes to this revision.Jul 12 2020, 9:30 AM
mehdi_amini added a reviewer: stephenneuendorffer.

I'd like to hear from @stephenneuendorffer about how this fits with D80358 ; we may want to assert on the type of Region, or use some sort of interface I'm not sure.

This revision now requires changes to proceed.Jul 12 2020, 9:32 AM

Looking at https://reviews.llvm.org/D80358, I don't see the definition of region arguments being entry block arguments changing.

And build failure are clang_tidy warnings for args_begin() etc functions. @mehdi_amini , how are these warnings currently suppressed in say Block.h? Or do we just ignore them? Also, based on the fact that definition of region entry block and its arguments being region arguments is not changing, can you unblock the change?

Looking at https://reviews.llvm.org/D80358, I don't see the definition of region arguments being entry block arguments changing.

Yeah I sync'd with Stephen today and there is no concern.

(you can ignore the clang-tidy warnings here)

mehdi_amini accepted this revision.Jul 13 2020, 7:55 PM
This revision is now accepted and ready to land.Jul 13 2020, 7:55 PM
This revision was automatically updated to reflect the committed changes.

Looking at https://reviews.llvm.org/D80358, I don't see the definition of region arguments being entry block arguments changing.

Yeah I sync'd with Stephen today and there is no concern.

Yes, I agree. I think it makes sense for Regions to have the same arguments as their blocks. However, the relationship between operation arguments and region arguments is something that is determined by the semantics of each individual operation. With FuncOp, the function arguments and region arguments have the same type and arity, but in general, they may not.