InstCombine greediness blocks subsequent optimizations

[manueljacob]

Bugzilla Link	28221
Version	trunk
OS	Linux
CC	@sanjoy,@rotateright

Extended Description

This is a more general case of Bug 27869.

This is the input IR: (1)

define zeroext i1 @​test(i1 %b, i32 %i) {
%conv1 = zext i1 %b to i32
%cmp = icmp slt i32 %i, 0
%conv2 = zext i1 %cmp to i32
%and = and i32 %conv1, %conv2
%tobool = icmp ne i32 %and, 0
ret i1 %tobool
}

This could be optimized to: (2)

define zeroext i1 @​test(i1 %b, i32 %i) {
%cmp = icmp slt i32 %i, 0
%res = and i1 %b, %cmp
ret i1 %res
}

But currently this is transformed to: (3)

define zeroext i1 @​test(i1 %b, i32 %i) {
%conv1 = zext i1 %b to i32
%i.lobit = lshr i32 %i, 31
%and = and i32 %i.lobit, %conv1
%tobool = icmp ne i32 %and, 0
ret i1 %tobool
}

InstCombine transforms the (zext (< i 0)) pattern into clever bit shifting: (lshr i 31). At first this saves one operation, but it obfuscates the intent of the code.

There are two problems:

a) InstCombine (or any other pass) doesn't transform (3) into (2).

b) Because InstCombine is greedy (combining instructions in depth-first order), the problematic IR in (3) is generated in the first place. If InstCombine deferred the (zext (< i 0)) -> (lshr i 31) transformation until later, (1) would have been transformed into (2).

[rotateright]

The problem is more general than just a sign-bit check. Instcombine converts all kinds of masked compares into shift and wider logic:

define i1 @​test2(i1 %b, i32 %x) {
%conv1 = zext i1 %b to i32
%and = and i32 %x, 8
%cmp = icmp eq i32 %and, 8
%conv2 = zext i1 %cmp to i32
%and2 = and i32 %conv1, %conv2
%tobool = icmp ne i32 %and2, 0
ret i1 %tobool
}

Becomes:

define i1 @​test2(i1 %b, i32 %x) {
%conv1 = zext i1 %b to i32
%shr = lshr i32 %x, 3
%and = and i32 %shr, %conv1
%tobool = icmp ne i32 %and, 0
ret i1 %tobool
}

But this seems better:

define i1 @​test3(i1 %b, i32 %x) {
%and1 = and i32 %x, 8
%cmp = icmp eq i32 %and1, 8
%and2 = and i1 %cmp, %b
ret i1 %and2
}

http://rise4fun.com/Alive/se

[rotateright]

We need a backend fix before we can fix this in InstCombine. The codegen for all of x86, aarch64, and powerpc64le looks better for the version with 'lshr' in the original problem description.

For my examples in comment 1, the codegen is equivalent across those targets.