This is an archive of the discontinued LLVM Phabricator instance.

[mlir][LLVMIR] Add support for importing struct-type ConstantAggregate(Zero)
ClosedPublic

Authored by myhsu on Apr 25 2022, 9:42 AM.

Details

Summary

We simply translate struct-type ConstantAggregate(Zero) into a serious of llvm.insertvalue operations against a llvm.undef root. Note that this doesn't affect the original logics on translating vector/array-type ConstantAggregate values.
Related discussion: link.

This patch also moves importer test files from test/Target/LLVMIR into test/Target/LLVMIR/Import.

Diff Detail

Event Timeline

myhsu created this revision.Apr 25 2022, 9:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 25 2022, 9:42 AM
myhsu requested review of this revision.Apr 25 2022, 9:42 AM
ftynse accepted this revision.Apr 26 2022, 1:14 AM

Nice, thank you!

mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
513
524

I see the code around doesn't do this either, but isn't there a way to get some reasonable location info?

526

Please expand auto unless the type is obvious from RHS (e.g., RHS is a cast) or impractical to write (lambdas, iterators).

This revision is now accepted and ready to land.Apr 26 2022, 1:14 AM
myhsu updated this revision to Diff 425374.Apr 26 2022, 5:56 PM
myhsu marked 2 inline comments as done.

minor fixes

myhsu added inline comments.Apr 26 2022, 6:10 PM
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
524

I think for ConstantAggregate originating from global variable's initializer (which is the common case), we can use the DebugLoc from the associated GlobalVariable. Thus, one possible solution will be adding an additional DebugLoc type (or mlir::Location type) argument for processConstant, such that caller can supply additional location info if available (from GlobalVariable, for example).
I'm not sure I want to make such change in this patch though.