Incorrect code generation

[codebje]

Hi again,

This short section of code is incorrectly generated:

int foo(unsigned int size)
{
  if ((size & ~15) >= 100) return 1;
  else return 4;
}

With -O0 the output never performs either the __iand or the comparison. The relevant portion of the assembly output is:

    xor a, a
    ld  c, 1
    xor a, c
    bit 0, a
    jp  nz, .LBB0_2

Obviously, this test always succeeds: a is set to 1. .LBB0_2 branches to the return 1.

With -O1 or better it merely saves time on the above constant computation:

_foo:
    call    __frameset0
    ld  hl, 1
    pop ix
    ret

It looks like the known bits computation makes a bit of a blunder:

Try combining %5:_(s1) = G_ICMP intpred(ugt), %3:_(s24), %4:_
[1] Compute known bits: %2:_(s24) = G_CONSTANT i24 -16
[1] Computed for: %2:_(s24) = G_CONSTANT i24 -16
[1] Known: 0xFFFFFF
[1] Zero: 0xF
[1] One:  0xFFFFF0
[1] Compute known bits: %0:_(s24) = G_LOAD %1:_(p0) :: (invariant load 3 from %fixed-stack.0, align 1)
[1] Computed for: %0:_(s24) = G_LOAD %1:_(p0) :: (invariant load 3 from %fixed-stack.0, align 1)
[1] Known: 0x0
[1] Zero: 0x0
[1] One:  0x0
[0] Compute known bits: %3:_(s24) = G_AND %0:_, %2:_
[0] Computed for: %3:_(s24) = G_AND %0:_, %2:_
[0] Known: 0xF
[0] Zero: 0xF
[0] One:  0x0
[0] Compute known bits: %4:_(s24) = G_CONSTANT i24 99
[0] Computed for: %4:_(s24) = G_CONSTANT i24 99
[0] Known: 0xFFFFFF
[0] Zero: 0xFFFF9C
[0] One:  0x63
Creating: G_CONSTANT

Erasing: %5:_(s1) = G_ICMP intpred(ugt), %3:_(s24), %4:_

Erasing: %5:_(s1) = G_ICMP intpred(ugt), %3:_(s24), %4:_

Created: %5:_(s1) = G_CONSTANT i1 true

It appears to be the narrow_icmp combiner at fault. lib/CodeGen/GlobalISel/CombinerHelper.cpp:4035 XORs together the known bits from LHS and RHS, in this case 0xF/0x0 with 0xFFFF9C/0x63, producing 0xC/0x3. It checks if all bits are zero - they're not. It checks if any bits are one: two bits are, and so it sets the result to an immediate value of 1.

This check is only correct for direct equality comparisons, not for inequalities like ugt.

I added a small guard before the XOR to work around the problem in my fork, but I'm not sure if this is a particularly good way to deal with the issue:

  if (Pred != CmpInst::ICMP_EQ) {
      return false;
  }

[jacobly0]

Should be generalized correctly to relational comparisons now.