This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Introduce more intuitive wording for attributes.
ClosedPublic

Authored by silvas on Feb 4 2021, 4:50 PM.

Details

Summary

After discussion, it seems like we want to go with
"inherent/discardable". These seem to best capture the relationship with
the op semantics and don't conflict with other terms.

Please let me know your preferences. Some of the other contenders are:

"intrinsic" side | "annotation" side
-----------------+------------------
characteristic   | annotation
closed           | open
definitional     | advisory
essential        | discardable
expected         | unexpected
innate           | acquired
internal         | external
intrinsic        | extrinsic
known            | unknown
local            | global
native           | foreign
inherent         | acquired

Rationale:

  • discardable: good. discourages use for stable data.
  • inherent: good
  • annotation: redundant and doesn't convey difference
  • intrinsic: confusable with "compiler intrinsics".
  • definitional: too much of a mounthful
  • extrinsic: too exotic of a word and hard to say
  • acquired: doesn't convey the relationship to the semantics
  • internal/external: not immediately obvious: what is internal to what?
  • innate: similar to intrinsic but worse
  • acquired: we don't typically think of an op as "acquiring" things
  • known/unknown: by who?
  • local/global: to what?
  • native/foreign: to where?
  • advisory: confusing distinction: is the attribute itself advisory or is the information it provides advisory?
  • essential: an intrinsic attribute need not be present.
  • expected: same issue as essential
  • unexpected: by who/what?
  • closed/open: whether the set is open or closed doesn't seem essential to the attribute being intrinsic. Also, in theory an op can have an unbounded set of intrinsic attributes (e.g. arg<N> for func).
  • characteristic: unless you have a math background this probably doesn't make as much sense

Diff Detail

Event Timeline

silvas created this revision.Feb 4 2021, 4:50 PM
silvas requested review of this revision.Feb 4 2021, 4:50 PM
silvas added a reviewer: ftynse.
silvas updated this revision to Diff 321619.Feb 4 2021, 4:54 PM

slight tweak

silvas edited the summary of this revision. (Show Details)Feb 4 2021, 4:54 PM
silvas edited the summary of this revision. (Show Details)Feb 4 2021, 4:57 PM
jdd added a subscriber: jdd.Feb 4 2021, 5:32 PM

“Intrinsic” makes me think of compiler intrinsics, which this is not. I like “property” since they are necessary properties of ops. I don’t have a strong preference other than the rejection reasons you give.

@jdd raises a good point about "intrinsic" being overloaded, which would tilt the scale towards "inherent" instead for me.

silvas added a comment.Feb 4 2021, 5:56 PM
In D96093#2543846, @jdd wrote:

“Intrinsic” makes me think of compiler intrinsics, which this is not. I like “property” since they are necessary properties of ops. I don’t have a strong preference other than the rejection reasons you give.

Agreed the clash with the notion of "compiler intrinsics" is a bit unfortunate. "inherent" is a bit better in that regard, but doesn't read as well.

I don't like "property" because it is ambiguous between two meanings, and they aren't a terribly good fit anyway:

  1. the abstract notion of "property" of a thing (e.g. a dog has a property which is its "fur color")
  2. the object oriented notion of property (e.g. related to getters, setters) which is just an abstract slot on an object.

I reject 1) because frequently a single "abstract property" is spread across multiple different intrinsic attributes. E.g. you might have 2 parallel array attributes that relate to each other and overall define an abstract property, but individually they do not. E.g. "foo.dog" { fur_color_regions = ["head", "body", "tail"], fur_colors = ["black", "brown", "spotted"] }.

I reject 2) because "slot on an object" doesn't really carry much information in this context -- I would prefer something that suggests "the op has special knowledge of these attributes" somehow.

I think that "inherent" is a bit more specific than "intrinsic" and avoids confusion with llvm intrinsics etc, but it is a slight difference either way.

I don't think that "annotation attributes" works well. That seems redundant and doesn't convey the difference here well. I'd go with something like: discardable, foreign, unknown, external, extensible, etc. I am not sure what the concern about negative connotations is. If anything, we *want* to discourage people from using these for stable data, that's the whole idea :-)

mlir/docs/LangRef.md
305

s/dim/std.dim/ ?

bondhugula added a subscriber: bondhugula.EditedFeb 4 2021, 6:29 PM

"inherent" (or "core") and "external" are looking good to me. Thanks!

silvas updated this revision to Diff 321868.Feb 5 2021, 1:44 PM

Switch to inherent/discardable.

silvas edited the summary of this revision. (Show Details)Feb 5 2021, 1:44 PM
silvas updated this revision to Diff 321870.Feb 5 2021, 1:45 PM

minor wording tweak

mehdi_amini accepted this revision.Feb 5 2021, 2:48 PM
This revision is now accepted and ready to land.Feb 5 2021, 2:48 PM

(please wait for at least one more LGTM)

rriddle accepted this revision.Feb 5 2021, 3:47 PM

LGTM. +1 on inherent/discardable

mlir/docs/LangRef.md
1318
silvas updated this revision to Diff 321909.Feb 5 2021, 6:26 PM
silvas marked an inline comment as done.

Address River's comment

This revision was automatically updated to reflect the committed changes.

This is great, thank you again for clarifying this!