This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Move data layout from LLVMDialect to module Op attributes
ClosedPublic

Authored by ftynse on Aug 10 2020, 7:05 AM.

Details

Summary

Legacy implementation of the LLVM dialect in MLIR contained an instance of
llvm::Module as it was required to parse LLVM IR types. The access to the data
layout of this module was exposed to the users for convenience, but in practice
this layout has always been the default one obtained by parsing an empty layout
description string. Current implementation of the dialect no longer relies on
wrapping LLVM IR types, but it kept an instance of DataLayout for
compatibility. This effectively forces a single data layout to be used across
all modules in a given MLIR context, which is not desirable. Remove DataLayout
from the LLVM dialect and attach it as a module attribute instead. Since MLIR
does not yet have support for data layouts, use the LLVM DataLayout in string
form with verification inside MLIR. Introduce the layout when converting a
module to the LLVM dialect and keep the default "" description for
compatibility.

This approach should be replaced with a proper MLIR-based data layout when it
becomes available, but provides an immediate solution to compiling modules with
different layouts, e.g. for GPUs.

This removes the need for LLVMDialectImpl, which is also removed.

Depends On D85650

Diff Detail

Event Timeline

ftynse created this revision.Aug 10 2020, 7:05 AM
Herald added a project: Restricted Project. · View Herald Transcript
ftynse requested review of this revision.Aug 10 2020, 7:05 AM
ftynse updated this revision to Diff 284418.Aug 10 2020, 10:02 AM

Forgot to git-add...

Does this have impact on our default JIT pipeline?

mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp
129

can we remove the TODO here
(it is still true, but it will probably be transparent at this point, right?)

Does this have impact on our default JIT pipeline?

Immediately, no, since the data layout has always being the default-constructed object and the execution engine was blindly resetting that to the current arch. If we keep it as llvm.data_layout = "", we will obtain exactly the same behavior. But now we can also set the proper data layout earlier in the process and use it in transformations.

mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp
129

It still depends on the translation, but arguably it should not. This TODO can be removed when we can ask MLIR about the preferred alignment for a vector instead of asking LLVM.

ftynse updated this revision to Diff 285041.Aug 12 2020, 4:47 AM

clang-tidy

aartbik accepted this revision.Aug 12 2020, 7:50 AM
This revision is now accepted and ready to land.Aug 12 2020, 7:50 AM