[TypeSize] Extend UnivariateLinearPolyBase with getWithIncrement/Decrement methods

[sdesmalen]

Summary

This patch adds getWithIncrement/getWithDecrement methods to
ElementCount and TypeSize to allow:

TypeSize::getFixed(8).getWithIncrement(8)     <=> TypeSize::getFixed(16)
TypeSize::getFixed(16).getWithDecrement(8)    <=> TypeSize::getFixed(8)
TypeSize::getScalable(8).getWithIncrement(8)  <=> TypeSize::getScalable(16)
TypeSize::getScalable(16).getWithDecrement(8) <=> TypeSize::getScalable(8)

This patch implements parts of the POC in D90342.

[dmgreen]

Can you just overload the + operator? Would that make the calling code more confusing?

I don't know this class very well so feel free to ignore me but I feel like it should be called "getWithIncrement" or something like it. That would be more like Type methods I've seen in the past like getWithNewBitwidth. Like I said if you are going for something else feel free to ignore me though.

[sdesmalen]

In D90713#2373445, @dmgreen wrote:

Can you just overload the + operator? Would that make the calling code more confusing?

In our SVE/SVE2 sync-up calls we settled on the approach of not using overloaded operators if the operation is not mathematically correct, and instead use named methods. This was the reason for removing operator/ from TypeSize and replacing it with divideCoefficientBy, because it only divides the (compile-time known) minimum value/coefficient and not the full runtime value in (coefficient * vscale) / RHS.

I don't know this class very well so feel free to ignore me but I feel like it should be called "getWithIncrement" or something like it. That would be more like Type methods I've seen in the past like getWithNewBitwidth. Like I said if you are going for something else feel free to ignore me though.

I like getWithIncrement, it seems a better name than add which indeed feels too similar to operator+. I wasn't able to think of a better name for it and therefore went with add, but I like this better. Thanks for the suggestion!

[sdesmalen]

• s/add/getWithIncrement/
• s/sub/getWithDecrement/

[dmgreen]

llvm/include/llvm/Support/TypeSize.h

235

I'm pretty sure my C++ has gotten worse since I started working on compilers.

Is the static_cast here needed? Will that end up just calling the new conversion constructor. Can it just create a LeafTy directly?

[sdesmalen]

llvm/include/llvm/Support/TypeSize.h

235

That's a fair question, I played around a bit with the options here. In the end went for a static_cast because it otherwise forces LeafTy to have a constructor of a certain shape (i.e. of whatever is called in this function). Otherwise it depends on a conversion constructor from parent -> child class, which always has the same form.

[dmgreen]

OK fair enough. If the other folks here don't disagree, this LGTM.

[ctetreau]

looks good to me