llvm / llvm/llvm-project

is_ascii() codegen regression

Open
#209,216 2 comments 0 reactions 0 assignees View on GitHub
llvm:optimizations missed-optimization regression:23
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

```llvm
define i1 @is_ascii(ptr %ptr, i64 %len) {
start:
br label %loop

loop:
%remaining = phi i64 [ %len, %start ], [ %remaining.next, %loop.latch ]
%exit.cond = icmp eq i64 %remaining, 0
br i1 %exit.cond, label %exit, label %loop.latch

loop.latch:
%remaining.next = add nsw i64 %remaining, -1
%gep = getelementptr inbounds nuw i8, ptr %ptr, i64 %remaining.next
%load = load i8, ptr %gep
%exit.cond.2 = icmp sgt i8 %load, -1
br i1 %exit.cond.2, label %loop, label %exit

exit:
ret i1 %exit.cond
}
```

After https://github.com/llvm/llvm-project/pull/187483 this gets rotated. The result of `-O2` is now (https://llvm.godbolt.org/z/8bGvTrjE5):

```llvm
define noundef i1 @is_ascii(ptr nofree readonly captures(none) %ptr, i64 %len) local_unnamed_addr #0 {
start:
%exit.cond1 = icmp eq i64 %len, 0
br i1 %exit.cond1, label %exit, label %loop.latch

loop.latch:
%remaining2 = phi i64 [ %remaining.next, %loop.latch ], [ %len, %start ]
%remaining.next = add nsw i64 %remaining2, -1
%gep = getelementptr inbounds nuw i8, ptr %ptr, i64 %remaining.next
%load = load i8, ptr %gep, align 1
%exit.cond.2 = icmp sgt i8 %load, -1
%exit.cond.2.not = xor i1 %exit.cond.2, true
%exit.cond = icmp eq i64 %remaining.next, 0
%or.cond = or i1 %exit.cond.2.not, %exit.cond
br i1 %or.cond, label %exit, label %loop.latch

exit:
%exit.cond.lcssa = phi i1 [ true, %start ], [ %exit.cond.2, %loop.latch ]
ret i1 %exit.cond.lcssa
}
```

This produces worse codegen (https://llvm.godbolt.org/z/YoWj54h4M). Old:

```asm
is_ascii: # @is_ascii
.LBB0_1: # %loop
movq %rsi, %rax
subq $1, %rsi
jb .LBB0_3
cmpb $0, -1(%rdi,%rax)
jns .LBB0_1
.LBB0_3: # %exit
testq %rax, %rax
sete %al
retq
```

New:

```asm
is_ascii: # @is_ascii
test rsi, rsi
je .LBB0_1
dec rsi
.LBB0_3: # %loop.latch
mov rax, rsi
sub rsi, 1
setb cl
cmp byte ptr [rdi + rax], 0
setns al
js .LBB0_5
test cl, cl
je .LBB0_3
.LBB0_5: # %exit
ret
.LBB0_1:
mov al, 1
ret
```

The additional check before the loop is an expected outcome of rotation, but the changes to the loop body are not great.

Contributor guide

Open the contributing guide

Research direction

Start with the is_ascii IR and reproduce its -O2 output using the linked Compiler Explorer cases; compare the loop rotation introduced by LLVM PR #187483 with the old and new assembly. Done means the loop-body codegen regression is resolved without losing the expected rotation behavior.

Written by the indexing model from the issue text.

Assessment

Domain
compilers, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.