llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.92k stars 11.52k forks source link

Failure to use x86 array arithmetic instead of manual add #46069

Open GabrielRavier opened 4 years ago

GabrielRavier commented 4 years ago
Bugzilla Link 46724
Version trunk
OS Linux
CC @topperc,@RKSimon,@rotateright

Extended Description

struct as
{
    as* a;
    as* b;
};

struct b
{
    as* a;
    as* b;
};

void f(b* a, as* s)
{
    if (a->a)
    {
        s->a = a->a;
        a->a->b = s;
    }
    else
    {
        a->b = s;
    }
}

With -O3, GCC outputs this :

f(b*, as*):
  mov rax, QWORD PTR [rdi]
  test rax, rax
  je .L2
  mov QWORD PTR [rsi], rax
  mov QWORD PTR [rax+8], rsi
  ret
.L2:
  mov QWORD PTR [rdi+8], rsi
  ret

LLVM outputs this :

f(b*, as*): # @f(b*, as*)
  mov rax, qword ptr [rdi]
  test rax, rax
  je .LBB0_2
  mov qword ptr [rsi], rax
  add rax, 8
  mov qword ptr [rax], rsi
  ret
.LBB0_2:
  add rdi, 8
  mov qword ptr [rdi], rsi
  ret
topperc commented 4 years ago

Code at the end of isel. Only a single store exists and the address comes from a phi. It gets copied later so that both paths can return separately instead of joining. But there nothing that can sink the address arithmetic at that point.

Function Live Ins: $rdi in %4, $rsi in %5

bb.0.entry:
  successors: %bb.2(0x30000000), %bb.1(0x50000000); %bb.2(37.50%), %bb.1(62.50%)
  liveins: $rdi, $rsi
  %5:gr64 = COPY $rsi
  %4:gr64 = COPY $rdi
  %0:gr64 = MOV64rm %4:gr64, 1, $noreg, 0, $noreg :: (load 8 from %ir.a111, !tbaa !2)
  TEST64rr %0:gr64, %0:gr64, implicit-def $eflags
  JCC_1 %bb.2, 4, implicit $eflags
  JMP_1 %bb.1

bb.1.if.then:
; predecessors: %bb.0
  successors: %bb.3(0x80000000); %bb.3(100.00%)

  MOV64mr %5:gr64, 1, $noreg, 0, $noreg, %0:gr64 :: (store 8 into %ir.a312, !tbaa !7)
  %1:gr64 = nuw ADD64ri8 %0:gr64(tied-def 0), 8, implicit-def dead $eflags
  JMP_1 %bb.3

bb.2.if.else:
; predecessors: %bb.0
  successors: %bb.3(0x80000000); %bb.3(100.00%)

  %2:gr64 = nuw ADD64ri8 %4:gr64(tied-def 0), 8, implicit-def dead $eflags

bb.3.if.end:
; predecessors: %bb.1, %bb.2

  %3:gr64 = PHI %1:gr64, %bb.1, %2:gr64, %bb.2
  MOV64mr %3:gr64, 1, $noreg, 0, $noreg, %5:gr64 :: (store 8 into %ir.b5.sink, !tbaa !9)
  RET 0
GabrielRavier commented 2 months ago

Seems to be fixed on trunk ?