[PATCH] Optimize redundant insertvalue instructions

[Hal Finkel]

----- Original Message -----
> From: "Michael Zolotukhin" <mzolotukhin at apple.com>
> To: "Hal J. Finkel" <hfinkel at anl.gov>
> Cc: "LLVM Commits" <llvm-commits at cs.uiuc.edu>
> Sent: Wednesday, May 7, 2014 7:07:35 AM
> Subject: Re: [PATCH] Optimize redundant insertvalue instructions
> 
> 
> 
> 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.

Fair enough.

> 
> Here is the new patch:
> 
> 

I don't actually see a call to visitInsertValueInst being inserted by the patch; I suppose you're putting it somewhere appropriate or else the test case would fail, right? Assuming that's the case, LGTM.

 -Hal

> 
> 
> 
> 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
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory