[llvm-commits] PATCH: new Value::getThruSplat API

[Nick Lewycky]

On 27 October 2010 10:57, Dan Gohman <gohman at apple.com> wrote:

>
> On Oct 26, 2010, at 11:34 PM, Nick Lewycky wrote:
>
> > This patch adds a new method on Value called getThruSplat() which returns
> the splatted value iff this is a ConstantValue with a splat, or 'this'
> otherwise. I demonstrate its usage by updating InstructionSimplify to
> uniformly handle splat vectors alongside ConstantInt values.
>
> Looks reasonable to me.
>
> How about "getScalarValue" instead of "getThruSplat"?
> I don't have strong opinions about it though.
>

ConstantArray and ConstantStruct aren't scalar and this method could return
those. For that matter, it could even return a vector, constant or not.

My concern is to make very sure that this is the API we really want. Are we
sure that "dyn_cast<ConstantInt>(V->getThruSplat())" doesn't do any more
work than the minimum (or at least, that the compiler can't optimize away)?
And is it okay to put this work on the lookup side? We will do this test
more often this way than if we did the work at the point of creation, which
is the pattern we follow in the rest of LLVM.

The most radical alternative API would be to factor out a common base that
represents a ConstantInt+ConstantVector(with splat) and another for
ConstantFP+ConstantVector(with splat), then teach ConstantVector to return
such a splatted CV from its getter. Sadly if you follow this through to its
conclusion, you end up using multiple inheritance in the Constant hierarchy
which causes all sorts of forms of pain.

I think the tradeoff in my patch is the most reasonable one, but I want to
be certain that we've thought of everything before I go off and apply this
idiom all over the place.

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20101027/d179e0aa/attachment.html>