This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Move casting method calls to function calls
ClosedPublic

Authored by tpopp on May 9 2023, 7:00 AM.

Details

Summary

The MLIR classes Type/Attribute/Operation/Op/Value support
cast/dyn_cast/isa/dyn_cast_or_null functionality through llvm's doCast
functionality in addition to defining methods with the same name.
This change continues the migration of uses of the method to the
corresponding function call as has been decided as more consistent.

This commit attempts to update all occurrences of the casts in .td
files, although it is likely that a couple were missed.

Context:

Implementation:
Unfortunatley, this was not automated, but was handled by mindlessly
going to next occurrences of patterns, selecting the piece of code to
be moved into the function call, and running a vim macro over the span
of around 4 hours.

Diff Detail

Event Timeline

tpopp created this revision.May 9 2023, 7:00 AM
tpopp requested review of this revision.May 9 2023, 7:00 AM
springerm accepted this revision.May 9 2023, 8:27 AM

Thanks for cleaning this up.

Is there a reason for writing ::llvm::cast instead of cast?

This revision is now accepted and ready to land.May 9 2023, 8:27 AM

Thanks for cleaning this up.

Is there a reason for writing ::llvm::cast instead of cast?

I have a stylistic and a practical reason for that choice.

Stylistic: I don't know if there is agreement on this, but a lot of tablegen based code uses fully resolved symbols, and I like to do that also, as the file itself does not know where it will be textually included (with what namespaces are being used or wrapping the include statement) later, and I don't like tying together the separate files more than necessary

Practical: If the code said cast and was inside a method of a class deriving from Type/Op/Attribute/etc, cast would resolve to the method, not the function, and then fail to compile from the wrong number of arguments. I didn't want to discover which subset fell into that situation as it would take much longer.

rriddle accepted this revision.May 12 2023, 1:52 AM