[PATCH] D42536: [AggresiveInstCombine] Added support of select and ShuffleVector to TruncInstCombine expression pattern

[Amjad Aboud via Phabricator via llvm-commits]

aaboud added a comment.

In https://reviews.llvm.org/D42536#988422, @spatel wrote:

> That's too easy - instcombine gets that. I think you need at least one more intermediate binop to show a case that instcombine can't handle.


You are right, I forgot that instcombine algorithm can handle zext/sext with multi use in case they have same type as trunc instruction destination type.
Here is a better test suggestion:

  define void @multi_uses_add(i32 %X) {
    %A = zext i32 %X to i64
    %B = add i64 %A, 5
    %M1 = mul i64 %B, 66
    %M2 = mul i64 %B, %A
    %C = and i64 %M1, %M2
    %T = trunc i64 %C to i32
    call i32 @use32(i32 %T)
    ret void
  }



> We need to make sure aggressive-instcombine is showing its unique capability in each test. If you agree that the case with a repeated op is just an oversight for instcombine, lets:
> 
> 1. Move the existing tests over to instcombine
> 2. Fix instcombine

I agree that we should improve instcombine where ever it make sense, such as this case, but I am afraid that I have no time to do it myself.
So, if you would like to take this task, I will be happy to review the code.

> 3. Add more complex tests for aggressive-instcombine's current functionality

Do you agree to the above example? I can duplicate it to handle all other instructions.

> 4. Enhance aggressive-instcombine for more opcodes/patterns.

I will be adding more opcodes that trunc expression should support, and with each new opcode I will be adding more tests.
Also, I already added few tests that were driven from implementation bugs due to missing handle of constant expressions.
see https://reviews.llvm.org/D42622.


https://reviews.llvm.org/D42536