[Local] Keep K's range if K does not move when combining metadata.

[fhahn]

Summary

As K has to dominate I, IIUC I's range metadata must be a subset of
K's. After Eli's recent clarification to the LangRef, loading a value
outside of the range is undefined behavior.
Therefore if I's range contains elements outside of K's range and we would load
one such value, K would cause undefined behavior.

In cases like hoisting/sinking, we still want the most generic range
over all code paths to/from the hoist/sink point. As suggested in the
patches related to D47339, I will refactor the handling of those
scenarios and try to decouple it from this function as follow up, once
we switched to a similar handling of metadata in most of
combineMetadata.

I updated some tests checking mostly the merging of metadata to keep the
metadata of to dominating load. The most interesting one is probably test8 in
test/Transforms/JumpThreading/thread-loads.ll. It contained a comment
about the alias metadata preventing us to eliminate the branch, but it
seem like the actual problem currently is that we merge the ranges of
both loads and cannot eliminate the icmp afterwards. With this patch, we
manage to eliminate the icmp, as the range of the first load excludes 8.

[fhahn]

ping

[fhahn]

ping

[fhahn]

ping

[fhahn]

ping

[fhahn]

ping

[efriedma]

LGTM; sorry about the delay.

[fhahn]

Thank you very much Eli! I'll take a look at the other metadata kinds as well.

lib/Transforms/Utils/Local.cpp

switch (Kind) {
       case LLVMContext::MD_alias_scope:
         K->setMetadata(Kind, MDNode::getMostGenericAliasScope(JMD, KMD));
         break;
       case LLVMContext::MD_noalias:
       case LLVMContext::MD_mem_parallel_loop_access:
         K->setMetadata(Kind, MDNode::intersect(JMD, KMD));
         break;
       case LLVMContext::MD_range:
+
+        // If K does move, use most generic range. Otherwise keep the range of
+        // K.
+        if (DoesKMove)
+          // FIXME: If K does move, we should drop the range info and nonnull.
+          //        Currently this function is used with DoesKMove in passes
+          //        doing hoisting/sinking and the current behavior of using the
+          //        most generic range is correct in those cases.
           K->setMetadata(Kind, MDNode::getMostGenericRange(JMD, KMD));
         break;
       case LLVMContext::MD_fpmath:
         K->setMetadata(Kind, MDNode::getMostGenericFPMath(JMD, KMD));
         break;
       case LLVMContext::MD_invariant_load:
         // Only set the !invariant.load if it is present in both instructions.
         K->setMetadata(Kind, JMD);
         break;

test/Transforms/GVN/range.ll

 ; RUN: opt -basicaa -gvn -S < %s | FileCheck %s
 
 define i32 @test1(i32* %p) {
 ; CHECK-LABEL: @test1(i32* %p)
-; CHECK: %a = load i32, i32* %p, !range !0
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE0:[0-9]+]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !0
   %b = load i32, i32* %p, !range !0
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test2(i32* %p) {
 ; CHECK-LABEL: @test2(i32* %p)
-; CHECK: %a = load i32, i32* %p
-; CHECK-NOT: range
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE0]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !0
   %b = load i32, i32* %p
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test3(i32* %p) {
 ; CHECK-LABEL: @test3(i32* %p)
-; CHECK: %a = load i32, i32* %p, !range ![[DISJOINT_RANGE:[0-9]+]]
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE0]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !0
   %b = load i32, i32* %p, !range !1
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test4(i32* %p) {
 ; CHECK-LABEL: @test4(i32* %p)
-; CHECK: %a = load i32, i32* %p, !range ![[MERGED_RANGE:[0-9]+]]
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE0]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !0
   %b = load i32, i32* %p, !range !2
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test5(i32* %p) {
 ; CHECK-LABEL: @test5(i32* %p)
-; CHECK: %a = load i32, i32* %p, !range ![[MERGED_SIGNED_RANGE:[0-9]+]]
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE3:[0-9]+]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !3
   %b = load i32, i32* %p, !range !4
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test6(i32* %p) {
 ; CHECK-LABEL: @test6(i32* %p)
-; CHECK: %a = load i32, i32* %p, !range ![[MERGED_TEST6:[0-9]+]]
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE5:[0-9]+]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !5
   %b = load i32, i32* %p, !range !6
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test7(i32* %p) {
 ; CHECK-LABEL: @test7(i32* %p)
-; CHECK: %a = load i32, i32* %p, !range ![[MERGED_TEST7:[0-9]+]]
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE7:[0-9]+]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !7
   %b = load i32, i32* %p, !range !8
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test8(i32* %p) {
 ; CHECK-LABEL: @test8(i32* %p)
-; CHECK: %a = load i32, i32* %p
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE9:[0-9]+]]
 ; CHECK-NOT: range
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !9
   %b = load i32, i32* %p, !range !10
   %c = add i32 %a, %b
   ret i32 %c
 }
 
-; CHECK: ![[DISJOINT_RANGE]] = !{i32 0, i32 2, i32 3, i32 5}
-; CHECK: ![[MERGED_RANGE]] = !{i32 0, i32 5}
-; CHECK: ![[MERGED_SIGNED_RANGE]] = !{i32 -5, i32 -2, i32 1, i32 5}
-; CHECK: ![[MERGED_TEST6]] = !{i32 10, i32 1}
-; CHECK: ![[MERGED_TEST7]] = !{i32 3, i32 4, i32 5, i32 2}
+; CHECK: ![[RANGE0]] = !{i32 0, i32 2}
+; CHECK: ![[RANGE3]] = !{i32 -5, i32 -2}
+; CHECK: ![[RANGE5]] = !{i32 10, i32 1}
+; CHECK: ![[RANGE7]] = !{i32 1, i32 2, i32 3, i32 4}
+; CHECK: ![[RANGE9]] = !{i32 1, i32 5}
 
 !0 = !{i32 0, i32 2}
 !1 = !{i32 3, i32 5}
 !2 = !{i32 2, i32 5}
 !3 = !{i32 -5, i32 -2}
 !4 = !{i32 1, i32 5}
 !5 = !{i32 10, i32 1}
 !6 = !{i32 12, i32 16}
 !7 = !{i32 1, i32 2, i32 3, i32 4}
 !8 = !{i32 5, i32 1}
 !9 = !{i32 1, i32 5}
 !10 = !{i32 5, i32 1}

test/Transforms/InstCombine/load-combine-metadata.ll

 define void @test_load_load_combine_metadata(i32*, i32*, i32*) {
   %a = load i32, i32* %0, !tbaa !8, !range !0, !alias.scope !5, !noalias !6
   %b = load i32, i32* %0, !tbaa !8, !range !1
   store i32 %a, i32* %1
   store i32 %b, i32* %2
   ret void
 }
 
-; CHECK: ![[RANGE]] = !{i32 0, i32 5, i32 7, i32 9}
+; CHECK: ![[RANGE]] = !{i32 0, i32 5}
 !0 = !{ i32 0, i32 5 }
 !1 = !{ i32 7, i32 9 }
 !2 = !{!2}
 !3 = !{!3, !2}
 !4 = !{!4, !2}
 !5 = !{!3}
 !6 = !{!4}
 !7 = !{ !"tbaa root" }
 !8 = !{ !9, !9, i64 0 }
 !9 = !{ !"scalar type", !7}

test/Transforms/JumpThreading/thread-loads.ll

 bb2:
   %v4 = tail call i32 (...) @f2()
   ret i32 %res.0
 
 bb3:
   ret i32 %res.0
 }
 
-; Make sure we merge the aliasing metadata. (If we don't, we have a load
-; with the wrong metadata, so the branch gets incorrectly eliminated.)
+; Make sure we merge the aliasing metadata. We keep the range metadata for the
+; first load, as it dominates the second load. Hence we can eliminate the
+; branch.
 define void @test8(i32*, i32*, i32*) {
 ; CHECK-LABEL: @test8(
-; CHECK: %a = load i32, i32* %0, !range !4
+; CHECK: %a = load i32, i32* %0, !range ![[RANGE4:[0-9]+]]
 ; CHECK-NEXT: store i32 %a
-; CHECK: br i1 %c
+; CHECK-NEXT: %xxx = tail call i32 (...) @f1()
+; CHECK-NEXT: ret void
   %a = load i32, i32* %0, !tbaa !0, !range !4, !alias.scope !9, !noalias !10
   %b = load i32, i32* %0, !range !5
   store i32 %a, i32* %1
   %c = icmp eq i32 %b, 8
   br i1 %c, label %ret1, label %ret2
 
 ret1:
   ret void

 
 left_x:
   ret i32 20
 
 right_x:
   ret i32 10
 }
 
+; CHECK: ![[RANGE4]] = !{i32 0, i32 1}
+
 !0 = !{!3, !3, i64 0}
 !1 = !{!"omnipotent char", !2}
 !2 = !{!"Simple C/C++ TBAA"}
 !3 = !{!"int", !1}
 !4 = !{ i32 0, i32 1 }
 !5 = !{ i32 8, i32 10 }
 !6 = !{!6}
 !7 = !{!7, !6}
 !8 = !{!8, !6}
 !9 = !{!7}
 !10 = !{!8}

test/Transforms/NewGVN/range.ll

 ; RUN: opt -basicaa -newgvn -S < %s | FileCheck %s
 
 define i32 @test1(i32* %p) {
 ; CHECK-LABEL: @test1(i32* %p)
-; CHECK: %a = load i32, i32* %p, !range !0
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE0:[0-9]+]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !0
   %b = load i32, i32* %p, !range !0
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test2(i32* %p) {
 ; CHECK-LABEL: @test2(i32* %p)
-; CHECK: %a = load i32, i32* %p
-; CHECK-NOT: range
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE0]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !0
   %b = load i32, i32* %p
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test3(i32* %p) {
 ; CHECK-LABEL: @test3(i32* %p)
-; CHECK: %a = load i32, i32* %p, !range ![[DISJOINT_RANGE:[0-9]+]]
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE0]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !0
   %b = load i32, i32* %p, !range !1
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test4(i32* %p) {
 ; CHECK-LABEL: @test4(i32* %p)
-; CHECK: %a = load i32, i32* %p, !range ![[MERGED_RANGE:[0-9]+]]
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE0]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !0
   %b = load i32, i32* %p, !range !2
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test5(i32* %p) {
 ; CHECK-LABEL: @test5(i32* %p)
-; CHECK: %a = load i32, i32* %p, !range ![[MERGED_SIGNED_RANGE:[0-9]+]]
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE3:[0-9]+]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !3
   %b = load i32, i32* %p, !range !4
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test6(i32* %p) {
 ; CHECK-LABEL: @test6(i32* %p)
-; CHECK: %a = load i32, i32* %p, !range ![[MERGED_TEST6:[0-9]+]]
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE5:[0-9]+]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !5
   %b = load i32, i32* %p, !range !6
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test7(i32* %p) {
 ; CHECK-LABEL: @test7(i32* %p)
-; CHECK: %a = load i32, i32* %p, !range ![[MERGED_TEST7:[0-9]+]]
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE7:[0-9]+]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !7
   %b = load i32, i32* %p, !range !8
   %c = add i32 %a, %b
   ret i32 %c
 }
 
 define i32 @test8(i32* %p) {
 ; CHECK-LABEL: @test8(i32* %p)
-; CHECK: %a = load i32, i32* %p
+; CHECK: %a = load i32, i32* %p, !range ![[RANGE9:[0-9]+]]
 ; CHECK-NOT: range
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, i32* %p, !range !9
   %b = load i32, i32* %p, !range !10
   %c = add i32 %a, %b
   ret i32 %c
 }
 
-; CHECK: ![[DISJOINT_RANGE]] = !{i32 0, i32 2, i32 3, i32 5}
-; CHECK: ![[MERGED_RANGE]] = !{i32 0, i32 5}
-; CHECK: ![[MERGED_SIGNED_RANGE]] = !{i32 -5, i32 -2, i32 1, i32 5}
-; CHECK: ![[MERGED_TEST6]] = !{i32 10, i32 1}
-; CHECK: ![[MERGED_TEST7]] = !{i32 3, i32 4, i32 5, i32 2}
+; CHECK: ![[RANGE0]] = !{i32 0, i32 2}
+; CHECK: ![[RANGE3]] = !{i32 -5, i32 -2}
+; CHECK: ![[RANGE5]] = !{i32 10, i32 1}
+; CHECK: ![[RANGE7]] = !{i32 1, i32 2, i32 3, i32 4}
+; CHECK: ![[RANGE9]] = !{i32 1, i32 5}
 
 !0 = !{i32 0, i32 2}
 !1 = !{i32 3, i32 5}
 !2 = !{i32 2, i32 5}
 !3 = !{i32 -5, i32 -2}
 !4 = !{i32 1, i32 5}
 !5 = !{i32 10, i32 1}
 !6 = !{i32 12, i32 16}
 !7 = !{i32 1, i32 2, i32 3, i32 4}
 !8 = !{i32 5, i32 1}
 !9 = !{i32 1, i32 5}
 !10 = !{i32 5, i32 1}