I couldn't reproduce the issue with these gn args:

is_clang = true
is_component_build = true
is_debug = false
symbol_level = 1
target_cpu = "x86"

Maybe it's a TOT regression.

I reproduced with my local clang, but it's a hang, not a crash, so it's not clear where we're miscompiling.

I think this might be caused by r281113. The conditional tail call optimization is firing on this code:
https://cs.chromium.org/chromium/src/chrome/browser/installable/installable_manager.cc?q=InstallableManager::IsComplete&sq=package:chromium&l=212&dr=CSs

Either one of the bools short circuits, or we call IsIconFetched.

There was a follow-up fix to the conditional tail call commit in r281223. But it seems the bot is still sad?

That revision doesn't fix the issue. I have a small reproducer that generates bad code. Some pass inserts a KILL of EAX/Al/etc before the RET, and so we end up returning false when we should return true.

If you run llc on that file, you get this:

"?IsComplete@InstallableManager@@ABE_NABUInstallableParams@@@Z": # @"\01?IsComplete@InstallableManager@@ABE_NABUInstallableParams@@@Z"
# BB#0:                                 # %entry
        movl    28(%ecx), %eax
        cmpb    $0, 440(%eax)
        je      LBB0_1
# BB#3:                                 # %land.lhs.true
        movl    4(%esp), %edx
        cmpb    $0, 8(%edx)
        je      LBB0_5
# BB#4:                                 # %lor.lhs.false
        movl    32(%ecx), %eax
        cmpb    $0, 5(%eax)
        je      LBB0_1
LBB0_5:                                 # %land.rhs
        cmpb    $0, 9(%edx)
        movb    $1, %al
        jne     "?IsIconFetched@InstallableManager@@ABE_NABUInstallableParams@@@Z" # TAILCALL
LBB0_1:
        xorl    %eax, %eax
LBB0_2:                                 # %land.end
                                        # kill: %AL<def> %AL<kill> %EAX<kill>
        retl    $4

If the 'jne' tail call falls through, we're supposed to return true, but instead we fall through the 'xorl %eax,%eax' instruction for mysterious reasons.

Deleted: iscomplete-fullopt.ll 11.9 KB

iscomplete-fullopt.ll 11.9 KB Download

Ah, maybe it's more on the theme of r281223, register masks being wrong.

After branch-folding on the reproducer:

BB#5: derived from LLVM BB %land.rhs
    Live Ins: %ECX %EDX
    Predecessors according to CFG: BB#3 BB#4
        %AL<def> = MOV8ri 1, %EAX<imp-def>
        CMP8mi %EDX<kill>, 1, %noreg, 9, %noreg, 0, %EFLAGS<imp-def>; mem:LD1[%fetch_valid_icon]
        TCRETURNdicc <ga:@"\01?IsIconFetched@InstallableManager@@ABE_NABUInstallableParams@@@Z">, 0, 9, <regmask %BH %BL %BP %BPL %BX %DI %DIL %EBP %EBX %EDI %ESI %SI %SIL>, %ESP<imp-use>
    Successors according to CFG: BB#2(0x30000000 / 0x80000000 = 37.50%)


There's no mention that TCRETURNdicc uses EFLAGS.

> There's no mention that TCRETURNdicc uses EFLAGS.

Fixing that doesn't seem to make this problem go away though..

It seems things go bad after machine block placement:

# *** IR Dump After Branch Probability Basic Block Placement ***:  
# Machine code for function ^A?IsComplete@InstallableManager@@ABE_NABUInstallableParams@@@Z: NoPHIs, TracksLiveness, NoVRegsFrame Objects:  
  fi#-1: size=4, align=4, fixed, at location [SP+4]  
Function Live Ins: %ECX  
  
BB#0: derived from LLVM BB %entry  
    Live Ins: %ECX  
        %EAX<def> = MOV32rm %ECX, 1, %noreg, 28, %noreg; mem:LD4[%_Myval2.i.i.i.i.i]  
        CMP8mi %EAX<kill>, 1, %noreg, 440, %noreg, 0, %EFLAGS<imp-def>; mem:LD1[%fetched](align=8)  
        JE_1 <BB#1>, %EFLAGS<imp-use>  
    Successors according to CFG: BB#1(0x30000000 / 0x80000000 = 37.50%) BB#3(0x50000000 / 0x80000000 = 62.50%)  
  
BB#3: derived from LLVM BB %land.lhs.true  
    Live Ins: %ECX  
    Predecessors according to CFG: BB#0  
        %EDX<def> = MOV32rm %ESP, 1, %noreg, 4, %noreg; mem:LD4[FixedStack-1]  
        CMP8mi %EDX, 1, %noreg, 8, %noreg, 0, %EFLAGS<imp-def>; mem:LD1[%check_installable](align=4)  
        JE_1 <BB#5>, %EFLAGS<imp-use>  
    Successors according to CFG: BB#5(0x30000000 / 0x80000000 = 37.50%) BB#4(0x50000000 / 0x80000000 = 62.50%)  
  
BB#4: derived from LLVM BB %lor.lhs.false  
    Live Ins: %ECX %EDX  
    Predecessors according to CFG: BB#3  
        %EAX<def> = MOV32rm %ECX, 1, %noreg, 32, %noreg; mem:LD4[%_Myval2.i.i.i.i.i11]  
        CMP8mi %EAX<kill>, 1, %noreg, 5, %noreg, 0, %EFLAGS<imp-def>; mem:LD1[%fetched4]  
        JE_1 <BB#1>, %EFLAGS<imp-use>  
    Successors according to CFG: BB#1(0x30000000 / 0x80000000 = 37.50%) BB#5(0x50000000 / 0x80000000 = 62.50%)  
  
BB#5: derived from LLVM BB %land.rhs  
    Live Ins: %ECX %EDX  
    Predecessors according to CFG: BB#3 BB#4  
        CMP8mi %EDX<kill>, 1, %noreg, 9, %noreg, 0, %EFLAGS<imp-def>; mem:LD1[%fetch_valid_icon]  
        %AL<def> = MOV8ri 1, %EAX<imp-def>  
        TAILJMPd_CC <ga:@"\01?IsIconFetched@InstallableManager@@ABE_NABUInstallableParams@@@Z">, 9, <regmask %BH %BL %BP %BPL %BX %DI %DIL %EBP %EBX %EDI %ESI %SI %SIL>, %ESP<imp-use>, %EFLAGS<imp-use,kill>, %ESP<imp-use>, %EFLAGS<imp-use>  
    Successors according to CFG: BB#2(0x30000000 / 0x80000000 = 37.50%)      <--- It says BB#2, but it really just falls through to BB#1 below!
  
BB#1:   
    Predecessors according to CFG: BB#0 BB#4  
        %EAX<def,tied1> = XOR32rr %EAX<undef,tied0>, %EAX<undef>, %EFLAGS<imp-def,dead>  
    Successors according to CFG: BB#2(?%)  
  
BB#2: derived from LLVM BB %land.end  
    Live Ins: %EAX  
    Predecessors according to CFG: BB#5 BB#1  
        %AL<def> = KILL %AL<kill>, %EAX<imp-use,kill>  
        RETIL 4, %AL<kill>

Aha, if I make sure isBarrier=0 on TAILJMPd in the tablegen file, that seems to fix the test case.

Is there anything we can revert?

I have a fix almost ready..

r281281

Looks fixed. CrWinClang went green in this build:
https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin%20tester/builds/6114

Thanks!