This is an archive of the discontinued LLVM Phabricator instance.

Differential D119325

[flang] Add type conversion for !fir.box<none>
ClosedPublic

Authored by clementval on Feb 9 2022, 5:14 AM.

Details

Reviewers
jeanPerier
kiranchandramohan
PeteSteinfeld
schweitz
svedanayagam
awarzynski
ftynse
Commits
rGfd0417a3cf7d: [flang] Add type conversion for !fir.box<none>
Summary

none is used in fir.box type to specify a polymorphic type.
This patch add the conversion from !fir.box<none> to LLVM.

This patch is part of the upstreaming effort from fir-dev branch.

Co-authored-by: Eric Schweitz <eschweitz@nvidia.com>
Co-authored-by: Jean Perier <jperier@nvidia.com>

Diff Detail

Event Timeline

clementval created this revision.Feb 9 2022, 5:14 AM
Herald added a reviewer: ftynse. · View Herald TranscriptFeb 9 2022, 5:14 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added subscribers: awarzynski, sdasgup3, wenzhicui and 20 others. · View Herald Transcript
clementval requested review of this revision.Feb 9 2022, 5:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2022, 5:14 AM
Herald added subscribers: stephenneuendorffer, nicolasvasilache, jdoerfert. · View Herald Transcript
ftynse added inline comments.Feb 9 2022, 5:16 AM
mlir/test/Conversion/StandardToLLVM/convert-types.mlir
35 ↗(On Diff #407125)

Spurious change.

awarzynski added inline comments.Feb 9 2022, 5:20 AM
flang/test/Fir/types-to-llvm.fir
428
clementval marked 2 inline comments as done.Feb 9 2022, 5:25 AM
clementval added inline comments.
flang/test/Fir/types-to-llvm.fir
428

Good catch!

mlir/test/Conversion/StandardToLLVM/convert-types.mlir
35 ↗(On Diff #407125)

My bad. Sorry for the noise.

clementval updated this revision to Diff 407126.Feb 9 2022, 5:25 AM
clementval marked 2 inline comments as done.

Address comments

Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2022, 5:25 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
ftynse resigned from this revision.Feb 9 2022, 5:29 AM
awarzynski accepted this revision.Feb 9 2022, 5:31 AM

LGTM, thanks!

flang/test/Fir/types-to-llvm.fir
424

Incomplete comment :)

This revision is now accepted and ready to land.Feb 9 2022, 5:31 AM
Harbormaster completed remote builds in B148458: Diff 407126.Feb 9 2022, 5:39 AM
Closed by commit rGfd0417a3cf7d: [flang] Add type conversion for !fir.box<none> (authored by clementval). · Explain WhyFeb 9 2022, 6:13 AM
This revision was automatically updated to reflect the committed changes.
clementval added a commit: rGfd0417a3cf7d: [flang] Add type conversion for !fir.box<none>.

Revision Contents

PathSize
flang/
lib/
Optimizer/
CodeGen/
4 lines
test/
Fir/
8 lines

Diff 407126

flang/lib/Optimizer/CodeGen/TypeConverter.h

Show First 20 Lines • Show All 118 Lines • ▼ Show 20 Lines addConversion([&](mlir::TupleType tuple) {
if (auto box = mem.dyn_cast<fir::BoxType>()) if (auto box = mem.dyn_cast<fir::BoxType>())
members.push_back(convertBoxTypeAsStruct(box)); members.push_back(convertBoxTypeAsStruct(box));
else else
members.push_back(convertType(mem).cast<mlir::Type>()); members.push_back(convertType(mem).cast<mlir::Type>());
} }
return mlir::LLVM::LLVMStructType::getLiteral(&getContext(), members, return mlir::LLVM::LLVMStructType::getLiteral(&getContext(), members,
/*isPacked=*/false); /*isPacked=*/false);
}); });
addConversion([&](mlir::NoneType none) {
return mlir::LLVM::LLVMStructType::getLiteral(
none.getContext(), llvm::None, /*isPacked=*/false);
});
} }
// i32 is used here because LLVM wants i32 constants when indexing into struct // i32 is used here because LLVM wants i32 constants when indexing into struct
// types. Indexing into other aggregate types is more flexible. // types. Indexing into other aggregate types is more flexible.
mlir::Type offsetType() { return mlir::IntegerType::get(&getContext(), 32); } mlir::Type offsetType() { return mlir::IntegerType::get(&getContext(), 32); }
// i64 can be used to index into aggregates like arrays // i64 can be used to index into aggregates like arrays
mlir::Type indexType() { return mlir::IntegerType::get(&getContext(), 64); } mlir::Type indexType() { return mlir::IntegerType::get(&getContext(), 64); }
▲ Show 20 Lines • Show All 232 Lines • Show Last 20 Lines

flang/test/Fir/types-to-llvm.fir

Show First 20 Lines • Show All 412 Lines • ▼ Show 20 Lines
// Test that fir.box inside tuple and derived type are lowered to struct type. // Test that fir.box inside tuple and derived type are lowered to struct type.
func private @foo0(%arg0: tuple<i64, !fir.box<i32>>) func private @foo0(%arg0: tuple<i64, !fir.box<i32>>)
// CHECK-LABEL: foo0 // CHECK-LABEL: foo0
// CHECK-SAME: !llvm.struct<(i64, struct<(ptr<i32>, i{{.*}})>)> // CHECK-SAME: !llvm.struct<(i64, struct<(ptr<i32>, i{{.*}})>)>
func private @foo1(%arg0: !fir.type<derived8{a:i64,b:!fir.box<i32>}>) func private @foo1(%arg0: !fir.type<derived8{a:i64,b:!fir.box<i32>}>)
// CHECK-LABEL: foo1 // CHECK-LABEL: foo1
// CHECK-SAME: !llvm.struct<"derived8", (i64, struct<(ptr<i32>, i{{.*}})>)> // CHECK-SAME: !llvm.struct<"derived8", (i64, struct<(ptr<i32>, i{{.*}})>)>
// -----
// Test that fir.box<none>
awarzynskiUnsubmitted
Not Done
ReplyInline Actions

Incomplete comment :)

awarzynski: Incomplete comment :)
// `none` is used for polymorphic type.
func private @foo0(%arg0: !fir.box<none>)
// CHECK-LABEL: foo0
// CHECK-SAME: !llvm.ptr<struct<(ptr<struct<()>>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}})>>)
awarzynskiUnsubmitted
Done
ReplyInline Actions
// CHECK-LABEL: foo0
- // CHECK-SAMEL !llvm.ptr<struct<(ptr<struct<()>>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}})>>)
+ // CHECK-SAME: !llvm.ptr<struct<(ptr<struct<()>>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}})>>)
awarzynski:
clementvalAuthorUnsubmitted
Done
ReplyInline Actions

Good catch!

clementval: Good catch!