[PATCH] D95449: [flang] Fix problems with constant arrays with lower bounds that are not 1

[Peter Klausler via Phabricator via llvm-commits]

klausler added a comment.

In D95449#2524183 <https://reviews.llvm.org/D95449#2524183>, @PeteSteinfeld wrote:

> In D95449#2523153 <https://reviews.llvm.org/D95449#2523153>, @klausler wrote:
>
>> Can you fix the problem by just calling "x.set_lbounds(GetLowerBounds())" on the folded result array and leaving ScalarConstantExpander alone?
>
> The expression that needs to have its lower bounds modified is of type Expr<SomeType>.  But the set_lbounds() method is defined for the types Constant<T>, ConstantBase<T>, and ConstantBounds<T>.  So to use set_lbounds(), I need to walk down through the layers of Expr<> objects to get to an object of type Constant<T> and then call set_lbounds() on it.
>
> So I could create a class to do this, but that class would be very similar to the exiting class that I've renamed as ArrayConstantMaker.  Potential differences would be that it might not require a method that takes Parenthese<T> as an arugment (I'm not sure about this), the call to Reshape() would not be required, and the new class would need fewer constructors.
>
> Thus, it seems to me that it's better to use a single class to create constant arrays with new lower bounds that handles both scalars and arrays.
>
> Let me know if my analysis is wrong, or if you agree with my analysis but not me conclusion.

I think I'd rather have two specifically purposed facilities for  scalar expansion and for setting lower bounds than one mash-up of those two.  If you must combine them, make sure that there's a CHECK that ensures that the original array is either scalar or has exactly the same shape.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95449/new/

https://reviews.llvm.org/D95449