[PATCH] Optimize redundant insertvalue instructions

[Michael Zolotukhin]

Hi Hal,

Thank you for the review, I updated the patch accordingly to your remarks.

There is a more general alternative to the cut-off: we could move from uses to defs, maintaining a bitvector, that indicates which fields were written. In this case the complexity would be O(N) and we will also catch partially overlapping accesses as well. I did something like this in my ‘sophisticated’ implementation. 

However, I’d prefer the most simple implementation, so I think that cut-off would be the best option here. I chose limit 10 - I don’t expect long chains of insertvalue instructions anyway, and it is enough for the original test.

Here is the new patch:

-------------- next part --------------
A non-text attachment was scrubbed...
Name: redundant-insertvalue-v2.patch
Type: application/octet-stream
Size: 3222 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140507/81805d54/attachment.obj>
-------------- next part --------------


Thanks,
Michael

On May 7, 2014, at 3:01 PM, Hal Finkel <hfinkel at anl.gov> wrote:

> Michael,
> 
> +/// Here the second instruction inserts values at the same indices, as the
> 
> +/// first one, making it redundant.
> 
> You mean, "making the first one redundant."
> 
> +  Value *V = dyn_cast<Value>(&I);
> 
> There should be no cast necessary here (and it certainly shouldn't be a dyn_cast).
> 
> +    V = dyn_cast<Value>(UserInsInst);
> 
> Same here.
> 
> +  while (V->hasOneUse()) {
> 
> Should we have a cut-off here? If this function is called on every InsertValue instruction, then this algorithm will be O(N^2) in the size of the InsertValue chain.
> 
> -Hal
> 
> ----- Original Message -----
>> From: "Michael Zolotukhin" <mzolotukhin at apple.com>
>> To: "LLVM Commits" <llvm-commits at cs.uiuc.edu>
>> Sent: Tuesday, May 6, 2014 9:42:12 AM
>> Subject: [PATCH] Optimize redundant insertvalue instructions
>> 
>> 
>> Hi,
>> 
>> 
>> This patch implements a simple optimization of redundant insertvalue
>> instructions.
>> 
>> 
>> Here is a small example where it will help:
>> 
>> %0 = insertvalue { i8, i32 } undef, i8 %x, 0
>> %1 = insertvalue { i8, i32 } %0, i8 %y, 0
>> 
>> 
>> In this example, the second instruction inserts values at the same
>> indices, as the first one, and consequently, the first instruction
>> is redundant.
>> This code will be transformed to:
>> 
>> %0 = insertvalue { i8, i32 } undef, i8 %y, 0
>> 
>> 
>> This optimization catches nothing on specs and test-suite, but the
>> example was inspired by a real world application.
>> 
>> 
>> As an experiment, I tried a more sophisticated implementation, which
>> was capable of dealing with several users and with partially
>> overlapping indices, but that also caught nothing on specs (and for
>> my application the simple version was enough). If there is an
>> interest, I could share a patch with this more complicated
>> implementation as well.
>> 
>> 
>> Is it ok to commit this patch?
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> Thanks,
>> Michael
> 
> -- 
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory