This is an archive of the discontinued LLVM Phabricator instance.

Differential D129264

[Flang] Use appropriate casting in getExtentFromTriplet to allow nullptr
AbandonedPublic

Authored by kiranchandramohan on Jul 7 2022, 1:56 AM.

Details

Reviewers
jeanPerier
clementval
peixin
sscalpone
Summary

Use dyn_cast_or_null in place of dyn_cast to allow the defining op to be
a nullptr in getExtentFromTriplet.
A testcase which crashes without this change is provided.

Diff Detail

Event Timeline

kiranchandramohan created this revision.Jul 7 2022, 1:56 AM
Herald added a reviewer: sscalpone. · View Herald TranscriptJul 7 2022, 1:56 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
Herald added a subscriber: mehdi_amini. · View Herald Transcript
kiranchandramohan requested review of this revision.Jul 7 2022, 1:56 AM
Herald added a subscriber: jdoerfert. · View Herald TranscriptJul 7 2022, 1:56 AM
clementval added a comment.EditedJul 7 2022, 1:59 AM

This was fixed a couple days ago in https://reviews.llvm.org/D129077

https://github.com/llvm/llvm-project/blob/519d7876cbee5a5d3cd40d41525cd45e44fb07a8/flang/lib/Optimizer/Builder/FIRBuilder.cpp#L1237

peixin added inline comments.Jul 7 2022, 2:00 AM
flang/lib/Optimizer/Builder/FIRBuilder.cpp
1238

You may should rebase on current main. This has been fixed by @clementval . https://github.com/llvm/llvm-project/blob/519d7876cbee5a5d3cd40d41525cd45e44fb07a8/flang/lib/Optimizer/Builder/FIRBuilder.cpp#L1237-L1241

dyn_cast_or_null looks better than isa_and_nonnull.

Harbormaster completed remote builds in B174086: Diff 442821.Jul 7 2022, 2:13 AM
kiranchandramohan abandoned this revision.Jul 7 2022, 2:20 AM

Ahh. I haven't rebased. Thanks @clementval @peixin. I will close.

Revision Contents

PathSize
flang/
lib/
Optimizer/
Builder/
3 lines
test/
Lower/
16 lines

Diff 442821

flang/lib/Optimizer/Builder/FIRBuilder.cpp

//===-- FIRBuilder.cpp ----------------------------------------------------===// //===-- FIRBuilder.cpp ----------------------------------------------------===//
Lint: Lint
Inline Actions

clang-format not found in user’s local PATH; not linting file.

Lint: Lint: clang-format not found in user’s local PATH; not linting file.
// //
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information. // See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
▲ Show 20 Lines • Show All 1,220 Lines • ▼ Show 20 Lines
llvm::Optional<std::int64_t> llvm::Optional<std::int64_t>
fir::factory::getExtentFromTriplet(mlir::Value lb, mlir::Value ub, fir::factory::getExtentFromTriplet(mlir::Value lb, mlir::Value ub,
mlir::Value stride) { mlir::Value stride) {
std::function<llvm::Optional<std::int64_t>(mlir::Value)> getConstantValue = std::function<llvm::Optional<std::int64_t>(mlir::Value)> getConstantValue =
[&](mlir::Value value) -> llvm::Optional<std::int64_t> { [&](mlir::Value value) -> llvm::Optional<std::int64_t> {
if (auto valInt = fir::factory::getIntIfConstant(value)) if (auto valInt = fir::factory::getIntIfConstant(value))
return valInt; return valInt;
else if (auto valOp = mlir::dyn_cast<fir::ConvertOp>(value.getDefiningOp())) else if (auto valOp =
mlir::dyn_cast_or_null<fir::ConvertOp>(value.getDefiningOp()))
peixinUnsubmitted
Not Done
ReplyInline Actions

You may should rebase on current main. This has been fixed by @clementval . https://github.com/llvm/llvm-project/blob/519d7876cbee5a5d3cd40d41525cd45e44fb07a8/flang/lib/Optimizer/Builder/FIRBuilder.cpp#L1237-L1241

dyn_cast_or_null looks better than isa_and_nonnull.

peixin: You may should rebase on current main. This has been fixed by @clementval . https://github.
return getConstantValue(valOp.getValue()); return getConstantValue(valOp.getValue());
return {}; return {};
}; };
if (auto lbInt = getConstantValue(lb)) { if (auto lbInt = getConstantValue(lb)) {
if (auto ubInt = getConstantValue(ub)) { if (auto ubInt = getConstantValue(ub)) {
if (auto strideInt = getConstantValue(stride)) { if (auto strideInt = getConstantValue(stride)) {
if (strideInt.value() != 0) { if (strideInt.value() != 0) {
std::int64_t extent = std::int64_t extent =
Show All 23 Lines

flang/test/Lower/getExtentFromTripletFix.f90

  • This file was added.
! Test that the get_extent_fix function works without crashing
! RUN: bbc -emit-fir %s -o - | FileCheck %s
! RUN: %flang_fc1 -emit-fir %s -o - | FileCheck %s
! CHECK-LABEL: func @_QPget_extent_fix
! CHECK: fir.do_loop
! CHECK: fir.call @_FortranAProductInteger
! CHECK: fir.do_loop
! CHECK: fir.array_merge_store
subroutine get_extent_fix(map)
integer, dimension(:) :: map
integer :: counter
map = (/ (product(map(:counter)), counter = 1, sizeof(map)) /)
end subroutine