Page MenuHomePhabricator

[ODS] Make the getType() method on a OneResult instruction return a specific type.

Authored by lattner on Dec 23 2020, 6:19 PM.



Implement Bug 46698, making ODS synthesize a getType() method that returns a
specific C++ class for OneResult methods where we know that class. This eliminates
a common source of casts in things like:


because we know that myOp always returns a FIRRTLType. This also encourages
op authors to type their results more tightly (which is also good for

I chose to implement this by splitting the OneResult trait into itself plus a
OneTypedResult trait, given that many things are using hasTrait<OneResult>
to conditionalize various logic.

While this changes makes many many ops get more specific getType() results, it
is generally drop-in compatible with the previous behavior because 'x.cast<T>()'
is allowed when x is already known to be a T. The one exception to this is that
we need declarations of the types used by ops, which is why a couple headers
needed additional #includes.

I updated a few things in tree to remove the now-redundant .cast<>'s, but there
are probably many more than can be removed.

Diff Detail

Event Timeline

lattner created this revision.Dec 23 2020, 6:19 PM
lattner requested review of this revision.Dec 23 2020, 6:19 PM
jpienaar accepted this revision.Dec 23 2020, 6:42 PM
jpienaar added a subscriber: jpienaar.

Nice, seems a good reduction


Why is this needed?


I was actually thinking of something else:

CastableType (better name welcome as always :))

This is one where the type constraint is a isa and you have the specific type. That probably does not subsume this - as one could have a more specific type but the isa would be a one of variant (which is possible due to variadic isa now) or least lax. But seems like a useful class where the predicate and the cppClassName are both known.


As you are here :) One space after period only (same below).


Nit: clang-format only changed parts to make these easier (you don't have to change, just for next one)

This revision is now accepted and ready to land.Dec 23 2020, 6:42 PM

Landed in 9eb3e564d3b, thanks!