Use module attribute to specify target arch.

[ienkovich]

This variant makes the target arch attribute mandatory for conversion to the LLVM dialect. We can fall back to the optional attribute if it looks too intrusive.

[etiotto]

Add an assert message pls.

[ienkovich]

Done

[etiotto]

Should we require the user to pass a ModuleOp rather than a generic operation ?

[ienkovich]

I thought accepting any nested op would be more convenient

[whitneywhtsang]

This variant makes the target arch attribute mandatory for conversion to the LLVM dialect. We can fall back to the optional attribute if it looks too intrusive.

My preference is to fall back to the optional attribute, as the original design is to rely on features attributes given either by PyTorch or ocloc query. One concern is by having the target arch attribute, passes will start to make assumptions based on that.

[ienkovich]

My preference is to fall back to the optional attribute, as the original design is to rely on features attributes given either by PyTorch or ocloc query. One concern is by having the target arch attribute, passes will start to make assumptions based on that.

This new attribute is orthogonal to the set of features. There is a set of features and a target arch that affects how you get access to those features.

We shouldn't use this attribute to assume HW features. I don't see how optionality can resolve your concern. We should just see that nobody tries to use the target arch attribute to make any feature assumptions because different target archs can be used for the same target HW.

[whitneywhtsang]

This new attribute is orthogonal to the set of features. There is a set of features and a target arch that affects how you get access to those features.

We shouldn't use this attribute to assume HW features. I don't see how optionality can resolve your concern. We should just see that nobody tries to use the target arch attribute to make any feature assumptions because different target archs can be used for the same target HW.

Having target arch attribute increases the chance one uses it to assume HW features, but we can try to prevent that with code reviews, so I am not against this change.

[ienkovich]

@whitneywhtsang @etiotto Layout-related tests fail because they use TTGIR as an input and it doesn't have the required attribute. I can add more test modifications to resolve it, but I think we better get back to the optional attribute. What do you think?