[PATCH] Fix GlobalOpt to use range metadata instead of creating SelectInsts.

[Dan Gohman]

On Wed, Mar 12, 2014 at 6:38 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> +      Constant *Cmp = ConstantExpr::getICmp(ICmpInst::ICMP_SLT,
> InitVal, OtherVal);
> +      Constant *One = ConstantInt::get(LI->getType(), 1);
> +      Value *Vals[] = {
> +        ConstantExpr::getSelect(Cmp, InitVal, OtherVal),
> +        ConstantExpr::getAdd(ConstantExpr::getSelect(Cmp, OtherVal,
> InitVal), One)
> +      };
> +      MDNode *MD = MDNode::get(LI->getContext(), Vals);
>
> This is assuming the comparisons folds. It would probably be a bit
> nicer to work with the integer values directly.
>

If the comparison doesn't fold and it remains a ConstantExpr, this code
still works. A ConstantExpr isn't as nice of a range bound as a
ConstantInt, but if that's all we have, that's all the select would have.
If we converted it to use integer values directly, or APInts rather, it
wouldn't handle that case. Also, the inputs and outputs are already both
Constants, and there's not that much code here.


> The comment
>
> ;; check that global opt turns integers that only hold 0 or 1 into bools
>
> is now out of date.
>

Attached is an updated patch which fixes this.


> The new optimization is a bit simpler and a bit less likely to upset
> other optimizations, but it is also a bit less powerful. Can you
> explain why you want the change? Have you hit a case where the select
> was detrimental?
>

Yes, I have a benchmark where the select disrupts other optimizations and
is detrimental. Also, it seems intuitively bad to speculatively create
selects hoping that it'll help subsequent optimizations more than it hurts.

This patch is still powerful enough to optimize the bzip2 benchmark that
originally motivated this optimization. It's only less powerful in that it
indicates a range of values instead of just the two discrete values, but it
seems most existing optimizations just care about the range. If more
precise information is desired, it would make sense to augment the range
metadata to be able to provide that. If that is done, then this
optimization could easily be augmented to support it.

Dan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140312/6f447780/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: use-range-metadata-instead-of-introducing-selects.patch
Type: text/x-patch
Size: 8945 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140312/6f447780/attachment.bin>