This is an archive of the discontinued LLVM Phabricator instance.

[flang] Add fir.select_type conversion to if-then-else ladder
ClosedPublic

Authored by clementval on Nov 18 2022, 2:44 AM.

Details

Summary

Convert fir.select_type operation to an if-then-else ladder.
The type guards are sorted before the conversion so it follows the
execution of SELECT TYPE construct as mentioned in 11.1.11.2 point 4
of the Fortran standard.

Depends on D138279

Diff Detail

Event Timeline

clementval created this revision.Nov 18 2022, 2:44 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
clementval requested review of this revision.Nov 18 2022, 2:44 AM

Remove unrelated change

Update getTypeCode

clementval added inline comments.Nov 18 2022, 5:43 AM
flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
439

This will be extracted and completed to it can be used here and in CodeGen. This will be done in a follow up patch.

Looks great, the class is ordering into a list to get the desired semantics is neat ! My main comment is that I think you might need to check if a type is an ancestor rather than a direct parent while doing this ordering.

flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
365–388

Is looking at the first parent name enough ?

What if you have class is with some type t3 and a classis with its grand parent t1, but not the parent t2 ? :

module m
type t1
  integer :: i
end type
type, extends(t1) :: t2
  integer :: j
end type
type, extends(t2) :: t3
  integer :: k
end type
contains
  subroutine select_type1(a)
    class(t1), intent(in) :: a
    select type (a)
    class is (t1)
      print*, 'class is t1'
    class is (t3)
      print*, 'class is t3'
    class default
      print*,'default'
    end select
  end subroutine
end module

 use m
 type(t3) :: a
 call select_type1(a)
end
396

It seems you could directly iterate through orderedTypeGuards vector rather than using and incrementing t.

403

Why does the unit attr case must have successor operands ?

clementval added inline comments.Nov 18 2022, 11:29 AM
flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
365–388

You are right. The current ordering is not enough. I need to reconstruct the whole ancestor chain to be able to query it. The fir.dispatch_table information currently available is enough but the ordering needs more work. I'll update the patch.

396

Right. Leftover from previous attempt without thinking twice :-)

clementval marked an inline comment as done.Nov 18 2022, 11:48 AM

Look at all the ancestors when reordering the CLASS IS type guards.

jeanPerier accepted this revision.Nov 21 2022, 12:47 AM

One more question, otherwise looks great.

flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
403

I still do not get why destOps.value() can be done under if (typeGuards[idx].dyn_cast<mlir::UnitAttr>()), but not in genTypeLadderStep.

This revision is now accepted and ready to land.Nov 21 2022, 12:47 AM
clementval added inline comments.Nov 21 2022, 12:55 AM
flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
403

It's actually wrong. The correct code is just rewriter.replaceOpWithNewOp<mlir::cf::BranchOp>(selectType, dest);. I'll update it before pushing.