phi inserted before landingpad by GCOVProfiling causes incorrect code generation

[Amanieu]

Bugzilla Link	45261
Resolution	FIXED
Resolved on	Apr 13, 2020 13:39
Version	9.0
OS	Linux
Blocks	#44654
Attachments	IR of the function with the bug
CC	@jdm,@MatzeB,@qcolombet,@rnk,@tstellar
Fixed by commit(s)	e5bf503 47e68d8

Extended Description

Upstream bug: rust-lang/rust#70148

rustc has a -Zprofile option which uses the "insert-gcov-profiling" LLVM pass to insert profiling instrumentation to a program.

This pass generates the following IR for landing pads:

catch.i: ; preds = %.noexc
%120 = phi i64* [ getelementptr inbounds ([24 x i64], [24 x i64]* @​__llvm_gcov_ctr.27, i64 0, i64 11), %.noexc ]
%121 = landingpad { i8*, i32 }
catch i8* null
%122 = load i64, i64* %120
%123 = add i64 %122, 1
store i64 %123, i64* %120

Which is then lowered (with llc -O0) to this:

.LBB0_14: # %catch.i
movq 160(%rsp), %rcx # 8-byte Reload
movl $1, %esi
.Ltmp2:
movabsq $__llvm_gcov_ctr.27, %rdi
addq $120, %rdi
movq (%rcx), %r8
addq $1, %r8
movq %r8, (%rcx)

The unwinding data points to .Ltmp2 as the landing pad address, and .LBB0_14 is unreachable. This results in %rcx being left uninitialized in the landing pad.

[rnk]

I can confirm, llc -O0 gives me the same IR. Removing -O0 makes the problem go away.

This sounds like a bad interaction between fastisel and sdisel.

In particular, the movl $1, %esi instruction looks like a local value, which might interact with local value sinking.

And, yes, the bug goes away when local value sinking is disabled:

$ llc -fast-isel-sink-local-values=0 -O0 reduced.ll -o - | grep -A4 '%catch.i'
.LBB0_14: # %catch.i
.Ltmp2:
movq 160(%rsp), %rcx # 8-byte Reload
movabsq $__llvm_gcov_ctr.27, %rsi
addq $120, %rsi

Right, it must be this code here:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp#L228

There are two relevant kinds of EH_LABEL here:

• labels around invokes
• labels for landingpad start

This code is looking for the invoke kind, not the landingpad kind.

I have no idea why we use EH_LABEL for landingpads. I think we could use basic block labels in LSDA tables instead, so we have one less label and MachineInstr to worry about. IIRC this EH_LABEL is a problem for other codegen passes as well.

[rnk]

Should be fixed by e5bf503.

But let's merge it to 10.0.1.

[tstellar]

Merged: 47e68d8

[tstellar]

mentioned in issue #44654