[Inliner] noalias metadata incorrectly propagated to caller instructions

[nikic]

Bugzilla Link	50270
Resolution	FIXED
Resolved on	May 15, 2021 05:56
Version	trunk
OS	Linux
Blocks	#48661
CC	@alex,@Dushistov,@dobbelaj-snps,@jrmuizel,@jdm,@jplatte,@tstellar
Fixed by commit(s)	aa9b02a 4eb7b15

Extended Description

declare { i64 } @​opaque_callee()

define { i64 } @​callee(i64 %x) {
%res = insertvalue { i64 } undef, i64 %x, 0
ret { i64 } %res
}

define i64 @​caller(i64* %p) {
%s = call { i64 } @​opaque_callee()
%x = extractvalue { i64 } %s, 0
call { i64 } @​callee(i64 %x), !noalias !​2
%y = load i64, i64* %p, !alias.scope !​2
ret i64 %y
}

!​0 = !{#0, !"domain"}
!​1 = !{#1, !​0, !"scope"}
!​2 = !{#1}

; RUN: opt -S -inline < %s

declare { i64 } @​opaque_callee()

define { i64 } @​callee(i64 %x) {
%res = insertvalue { i64 } undef, i64 %x, 0
ret { i64 } %res
}

define i64 @​caller(i64* %p) {
%s = call { i64 } @​opaque_callee(), !noalias !​0
%x = extractvalue { i64 } %s, 0
%y = load i64, i64* %p, align 4, !alias.scope !​0
ret i64 %y
}

!​0 = !{#1}
!​1 = distinct !{#1, !​2, !"scope"}
!​2 = distinct !{#2, !"domain"}

The !noalias metadata on the @​callee() call is propagated to the @​opaque_callee() call from the caller.

This happens because VMap may map to instructions from the caller if simplification occurred. The existing check in

llvm-project/llvm/lib/Transforms/Utils/InlineFunction.cpp

Lines 796 to 799 in d4bdeca

	// Check that key is an instruction, to skip the Argument mapping, which
	// points to an instruction in the original function, not the inlined one.
	if (!VMI->second || !isa<Instruction>(VMI->first))
	continue;

protects against a similar problem, but does not cover this case.

I believe the same basic problem also applies to a few other VMap walks inside the inliner.

This was originally reported at rust-lang/rust#84958.

[nikic]

assigned to @dobbelaj-snps

[nikic]

I believe this shows the same issue during metadata remapping:

declare { i64* } @​opaque_callee()

define { i64* } @​self_caller(i1 %c, i64* %a) {
br i1 %c, label %if, label %else

if:
%s = call { i64* } @​opaque_callee(), !noalias !​2
%x = extractvalue { i64* } %s, 0
%r = call { i64* } @​self_caller(i1 false, i64* %x)
ret { i64* } %r

else:
%r2 = insertvalue { i64* } undef, i64* %a, 0
load volatile i64, i64* %a, !alias.scope !​2
ret { i64* } %r2
}

!​0 = !{#0, !"domain"}
!​1 = !{#1, !​0, !"scope"}
!​2 = !{#1}

opt -inline results in:

declare { i64* } @​opaque_callee()

define { i64* } @​self_caller(i1 %c, i64* %a) {
br i1 %c, label %if, label %else

if: ; preds = %0
%s = call { i64* } @​opaque_callee(), !noalias !​0
%x = extractvalue { i64* } %s, 0
%1 = load volatile i64, i64* %x, align 4, !alias.scope !​0
ret { i64* } %s

else: ; preds = %0
%r2 = insertvalue { i64* } undef, i64* %a, 0
%2 = load volatile i64, i64* %a, align 4, !alias.scope !​3
ret { i64* } %r2
}

!​0 = !{#1}
!​1 = distinct !{#1, !​2, !"scope"}
!​2 = distinct !{#2, !"domain"}
!​3 = !{#4}
!​4 = distinct !{#4, !​5, !"scope"}
!​5 = distinct !{#5, !"domain"}

@​opaque_callee() here should have retained metadata !​3, rather than being assigned !​0.

This case is a bit trickier to produce, because we only remap metadata that is used in the callee, so this only happens if the caller and the callee are the same (or share metadata, but I think that's rather ill-defined).

[nikic]

Candidate patch: https://reviews.llvm.org/D102110

[nikic]

Fixed by aa9b02a on main. Keeping this issue open to track 12.0.1 backport.

[tstellar]

Hi Jeroen,

What is your opinion on backporting this?

https://reviews.llvm.org/rGaa9b02ac75350a6c7c949dd24d5c6a931be26ff9

[slacka]

After this fix, I also discovered that the OpenMP tests are failing on both Arch Linux 32 and i686 OpenSUSE Tumbleweed. Reported as Bug 50303.

[dobbelaj-snps]

Hi Jeroen,

What is your opinion on backporting this?

https://reviews.llvm.org/rGaa9b02ac75350a6c7c949dd24d5c6a931be26ff9

Hi Tom,

I do think we should backport this.
I just tried out the cherry-pick and it applies without any problem. The build and tests also went fine.

[tstellar]

Merged: 4eb7b15