Bounds check not eliminated

[jrmuizel]

Bugzilla Link	48544
Version	trunk
OS	All
CC	@fhahn,@RKSimon,@nikic

Extended Description

#include <stdlib.h>
char foo(char* data, size_t len, size_t i, size_t j) {
    if (i < len && j <= i) {
        if (j < len) {
            return data[j];
        } else {
            exit(1);
        }
    } else {
        return 0;
    }
}

compiles to:

foo(char*, unsigned long, unsigned long, unsigned long):
        cmp     rdx, rsi
        jnb     .L4
        cmp     rdx, rcx
        jb      .L4
        cmp     rsi, rcx
        jbe     .L3
        movzx   eax, BYTE PTR [rdi+rcx]
        ret
.L4:
        xor     eax, eax
        ret
.L3:
        push    rax
        mov     edi, 1
        call    exit

The call to exit should be eliminated.

[fhahn]

This is exactly a case targeted by the new ConstraintElimination pass added recently.

Building your example with -O3 -mllvm -enable-constraint-elimination gets rid of the check. (https://godbolt.org/z/839v98)

The pass is currently disabled by default, so any feedback & additional testing would be extremely valuable, in case you have code that has lots of such runtime checks.

[jrmuizel]

This test case is from rust-lang/rust#80075. I expect it will be interesting to see the results of this pass on Rust code.

[fhahn]

This test case is from rust-lang/rust#80075. I
expect it will be interesting to see the results of this pass on Rust code.

Ok great. I'd expect it to be valuable for languages that automatically insert/generate range checks. Unfortunately I am not set up to do any evaluation with Rust. Is this something you might be able to do? There's a stats counter constraint-elimination.NumCondsRemoved which indicates how often this transform fires.

It currently is also a bit dependent on the pass order. I am not sure which pipeline Rust uses, but it might need some tweaks to the pass to make it work more effectively on IR that Rust produces.

[jrmuizel]

Nikita might be better suited to trying that out.

[fhahn]

Yeah, it would be good to have a frontend that generates a lot of such runtime-checks to motivate enabling the pass by default. Any data on that would be very valuable to work towards flipping the default and to iron out potential issues.