Page MenuHomePhabricator

[mlir] replace LLVMIntegerType with built-in integer type

Authored by ftynse on Jan 6 2021, 7:19 AM.



The LLVM dialect type system has been closed until now, i.e. did not support
types from other dialects inside containers. While this has had obvious
benefits of deriving from a common base class, it has led to some simple types
being almost identical with the built-in types, namely integer and floating
point types. This in turn has led to a lot of larger-scale complexity: simple
types must still be converted, numerous operations that correspond to LLVM IR
intrinsics are replicated to produce versions operating on either LLVM dialect
or built-in types leading to quasi-duplicate dialects, lowering to the LLVM
dialect is essentially required to be one-shot because of type conversion, etc.
In this light, it is reasonable to trade off some local complexity in the
internal implementation of LLVM dialect types for removing larger-scale system
complexity. Previous commits to the LLVM dialect type system have adapted the
API to support types from other dialects.

Replace LLVMIntegerType with the built-in IntegerType plus additional checks
that such types are signless (these are isolated in a utility function that
replaced isa<LLVMType> and in the parser). Temporarily keep the possibility
to parse !llvm.i32 as a synonym for i32, but add a deprecation notice.

Diff Detail

Event Timeline

ftynse created this revision.Jan 6 2021, 7:19 AM
ftynse requested review of this revision.Jan 6 2021, 7:19 AM
Herald added a reviewer: herhut. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
antiagainst accepted this revision.Jan 6 2021, 8:20 AM


This revision is now accepted and ready to land.Jan 6 2021, 8:20 AM
silvas accepted this revision.Jan 6 2021, 11:25 AM

Awesome! Thank you for pushing this herculean effort!

mehdi_amini accepted this revision.Jan 6 2021, 2:35 PM
mehdi_amini added inline comments.

I'm not sure why we need to maintain this? This seems like an entire mechanical change?

This revision was landed with ongoing or failed builds.Jan 7 2021, 10:48 AM
This revision was automatically updated to reflect the committed changes.