[PATCH] D48828: [InstSimplify] fold extracting from std::pair

[Hiroshi Inoue via Phabricator via llvm-commits]

inouehrs added a comment.

> So i guess my question is, what in instcombine does this fold?
>  Is it very general, and this is only one of the cases it handles?
>  If not, maybe it should be refactored into instsimplify.

For the `shl->or->lshr` case, instcombine first identifies `or` is redundant and eliminates it. Then `shl` `shr` pair is eliminated. I think it is not general compared to my code.
For the `shl->or->and` case, instcombine does more general but more costly analysis in `SimplifyDemandedInstructionBits`. The same analysis seems too costly, but I expand the scope of my code for non-boolean cases.

> Can you explain that in layman terms?
>  I don't see anything phi-related in instsimplify changes.

To help jump threading, actually we need to optimize the code sequence spanning multiple BBs. For an example of `shl->or->and` case looks like

  BB1:
    %shl = shl nuw i64 %val, 32
    %or = or i64 %shl, 1
    br %BB2
  BB2:
    %phi = phi i64 [ %or, %BB1 ], ... 
    %and = and i64 %phi, 1

The current instcombine cannot optimize such cases.
My instsimplify patch does not handle phi by itself. The NewGVN calls instsimplify to check opportunities for simplifying instructions over phi.



================
Comment at: lib/Analysis/InstructionSimplify.cpp:1814
+  // from X and Y are disjoint in (X << A) | Y.
+  // ((X << A) | Y) & 1 -> Y  if effective width of Y is 1 (i.e. boolean).
+  // ((X << A) | Y) & (1 << A) -> X << A  if effective width of X is 1.
----------------
lebedev.ri wrote:
> I do not understand.
> Why is this only handling the case where `Y` is `bool`?
> 
I made the code more generic. What we really need to check is that this AND op selects all bits of X or Y, and no bit from the another.



https://reviews.llvm.org/D48828