This is an archive of the discontinued LLVM Phabricator instance.

[fir] Add utility function to FIRBuilder and MutableBox
ClosedPublic

Authored by clementval on Oct 21 2021, 2:41 AM.

Details

Summary

This patch is extracted from D111337 to make is smaller.
It introduce utility functions to the FIRBuilder and add the MutableBox
files.

  • genShape
  • readCharLen
  • getExtents

Diff Detail

Event Timeline

clementval created this revision.Oct 21 2021, 2:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 21 2021, 2:41 AM
clementval requested review of this revision.Oct 21 2021, 2:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 21 2021, 2:41 AM

LGTM. I have a few minor/trivial comments or questions.

flang/include/flang/Optimizer/Builder/MutableBox.h
127

Nit: fill the it -> fill it? Or something else is missing.

flang/lib/Optimizer/Builder/FIRBuilder.cpp
292

Nit:Can probably size this to exts.size()*2?

flang/lib/Optimizer/Builder/MutableBox.cpp
91

Nit: might be possible to set the size before entering the loop.

327

Nit: it -> It

358

Nit: an BoxValue -> a BoxValue

370

Nit: alloctables -> allocatables

374

Nit: Pointer -> Pointers

736

Nit: Is something missing towards the end?

flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp
360

Is the second version of genShape tested indirectly through this?
Might be good to have something with shapeshift. Or does testing that require codegen?

381

Can other variants (CharArrayBox,MutableArrayBox) be tested? Or do they require codegen?

This revision is now accepted and ready to land.Oct 21 2021, 10:33 AM
clementval marked 9 inline comments as done.Oct 21 2021, 1:45 PM
clementval added inline comments.
flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp
360

I have added a test with the shifts.

381

They will be easier to test when we have more lowering code upstream since they are not trival to create just for a test.

clementval marked an inline comment as done.

Address review comments