This is an archive of the discontinued LLVM Phabricator instance.

[NFC][flang] Remove unused code in lowerExplicitLowerBounds
ClosedPublic

Authored by peixin on Mar 22 2022, 8:16 PM.

Details

Summary

There is no need to lower the implicit lower bounds for assumed-shape
array in lowerExplicitLowerBounds. Remove the unused code.

Diff Detail

Event Timeline

peixin created this revision.Mar 22 2022, 8:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 22 2022, 8:16 PM
peixin requested review of this revision.Mar 22 2022, 8:16 PM
clementval added a comment.EditedMar 22 2022, 10:33 PM

Did you test that on fir-dev? Since we are in the middle of upstreaming I would rather not start removing code right now.

We are in the middle of upstreaming and this might be used later so I would prefer to not remove it now.

I also checked fir-dev. Actually, this is the incorrect code. Fortran 2018 section 8.5.8.3 point 3 is for assumed-shape array, then spec->lbound().isColon() should be true. If spec->lbound().isColon() is false, the array would be assumed-size array, and this function will get early return since box.lboundIsAllOnes() is true. That's why this code is never used.

If this patch affects the progress of upstreaming, we can wait for it.

We are in the middle of upstreaming and this might be used later so I would prefer to not remove it now.

I also checked fir-dev. Actually, this is the incorrect code. Fortran 2018 section 8.5.8.3 point 3 is for assumed-shape array, then spec->lbound().isColon() should be true. If spec->lbound().isColon() is false, the array would be assumed-size array, and this function will get early return since box.lboundIsAllOnes() is true. That's why this code is never used.

If this patch affects the progress of upstreaming, we can wait for it.

So please make the change in fir-dev first otherwise we might reupstream this later.

We are in the middle of upstreaming and this might be used later so I would prefer to not remove it now.

I also checked fir-dev. Actually, this is the incorrect code. Fortran 2018 section 8.5.8.3 point 3 is for assumed-shape array, then spec->lbound().isColon() should be true. If spec->lbound().isColon() is false, the array would be assumed-size array, and this function will get early return since box.lboundIsAllOnes() is true. That's why this code is never used.

If this patch affects the progress of upstreaming, we can wait for it.

So please make the change in fir-dev first otherwise we might reupstream this later.

OK. Got the rules. Submitted (https://github.com/flang-compiler/f18-llvm-project/pull/1545) in fir-dev.

We are in the middle of upstreaming and this might be used later so I would prefer to not remove it now.

I also checked fir-dev. Actually, this is the incorrect code. Fortran 2018 section 8.5.8.3 point 3 is for assumed-shape array, then spec->lbound().isColon() should be true. If spec->lbound().isColon() is false, the array would be assumed-size array, and this function will get early return since box.lboundIsAllOnes() is true. That's why this code is never used.

If this patch affects the progress of upstreaming, we can wait for it.

So please make the change in fir-dev first otherwise we might reupstream this later.

OK. Got the rules. Submitted (https://github.com/flang-compiler/f18-llvm-project/pull/1545) in fir-dev.

Thanks a lot. We should be done with this pretty soon.

This revision is now accepted and ready to land.Mar 23 2022, 9:17 AM

Thanks @clementval and @jeanPerier . I will land this after the one in fir-dev is mereged.

You can go ahead.