[GVN] Adjust metadata for coerced load CSE

[nikic]

Summary

When reusing a load in a way that requires coercion (i.e. casts or bit extraction) we currently fail to adjust metadata. Unfortunately, none of our existing tooling for this is really suitable, because combineMetadataForCSE() expects both loads to have the same type. In this case we may work on loads of different types and possibly and offset memory location.

As such, what this patch does is to simply drop all metadata, with the following exceptions:

• Metadata for which violation is known to always cause UB.
• If the load is !noundef, keep all metadata, as this will turn poison-generating metadata into UB as well.

This fixes the miscompile that was exposed by D146629.

[nikic]

Rebase

[StephenFan]

Is the behavior of violating !dereferencable or !dereferencable_or_null also IUB? If so, these metadata can also be retained.

[nikic]

In D148129#4264235, @StephenFan wrote:

Is the behavior of violating !dereferencable or !dereferencable_or_null also IUB? If so, these metadata can also be retained.

I think so, but this is currently not spelled out in LangRef and combineMetadata() doesn't treat it as such. I've put up https://reviews.llvm.org/D148202 to clarify this.

[nikic]

Preserve dereferenceable and dereferenceable_or_null.

[StephenFan]

llvm/lib/Transforms/Scalar/GVN.cpp

1008

For !noalias or !alias.scope, I think we don't care about the size and type. But it is also dropped. Maybe leave a TODO which says we can do better for these two metadata.

llvm/lib/Transforms/Scalar/GVN.cpp

if (Res->getType() != LoadTy) {
                         << "\n\n\n");
     }
   } else if (isCoercedLoadValue()) {
     LoadInst *CoercedLoad = getCoercedLoadValue();
     if (CoercedLoad->getType() == LoadTy && Offset == 0) {
       Res = CoercedLoad;
     } else {
       Res = getValueForLoad(CoercedLoad, Offset, LoadTy, InsertPt, DL);
+      // We are adding a new user for this load, for which the original
+      // metadata may not hold. Additionally, the new load may have a different
+      // size and type, so their metadata cannot be combined in any
+      // straightforward way.
+      // Drop all metadata that is not known to cause immediate UB on violation,
+      // unless the load has !noundef, in which case all metadata violations
+      // will be promoted to UB.
+      // TODO: We can combine noalias/alias.scope metadata here, because it is
+      // independent of the load type.
+      if (!CoercedLoad->hasMetadata(LLVMContext::MD_noundef))
+        CoercedLoad->dropUnknownNonDebugMetadata(
+            {LLVMContext::MD_dereferenceable,
+             LLVMContext::MD_dereferenceable_or_null,
+             LLVMContext::MD_invariant_load, LLVMContext::MD_invariant_group});
       LLVM_DEBUG(dbgs() << "GVN COERCED NONLOCAL LOAD:\nOffset: " << Offset
                         << "  " << *getCoercedLoadValue() << '\n'
                         << *Res << '\n'
                         << "\n\n\n");
     }
   } else if (isMemIntrinValue()) {
     Res = getMemInstValueForLoad(getMemIntrinValue(), Offset, LoadTy,
                                  InsertPt, DL);

llvm/test/Transforms/GVN/metadata.ll

;
   call void @use.ptr(ptr %a)
   call void @use.ptr(ptr %b)
   ret void
 }
 
 define void @load_ptr_nonnull_to_i64(ptr %p) {
 ; CHECK-LABEL: define void @load_ptr_nonnull_to_i64
 ; CHECK-SAME: (ptr [[P:%.*]]) {
-; CHECK-NEXT:    [[VAL:%.*]] = load ptr, ptr [[P]], align 8, !nonnull !6
+; CHECK-NEXT:    [[VAL:%.*]] = load ptr, ptr [[P]], align 8
 ; CHECK-NEXT:    [[VAL_INT:%.*]] = ptrtoint ptr [[VAL]] to i64
 ; CHECK-NEXT:    call void @use.i64(i64 [[VAL_INT]])
 ; CHECK-NEXT:    call void @use.i64(i64 [[VAL_INT]])
 ; CHECK-NEXT:    ret void
 ;
   %val = load ptr, ptr %p, align 8, !nonnull !{}
   %val.int = ptrtoint ptr %val to i64
   %val2 = load i64, ptr %p, align 8
;
   call void @use.i64(i64 %val.int)
   call void @use.i64(i64 %val2)
   ret void
 }
 
 define void @load_ptr_nonnull_to_i32(ptr %p) {
 ; CHECK-LABEL: define void @load_ptr_nonnull_to_i32
 ; CHECK-SAME: (ptr [[P:%.*]]) {
-; CHECK-NEXT:    [[VAL:%.*]] = load ptr, ptr [[P]], align 8, !nonnull !6
+; CHECK-NEXT:    [[VAL:%.*]] = load ptr, ptr [[P]], align 8
 ; CHECK-NEXT:    [[VAL_INT:%.*]] = ptrtoint ptr [[VAL]] to i64
 ; CHECK-NEXT:    [[TMP1:%.*]] = trunc i64 [[VAL_INT]] to i32
 ; CHECK-NEXT:    call void @use.i64(i64 [[VAL_INT]])
 ; CHECK-NEXT:    call void @use.i32(i32 [[TMP1]])
 ; CHECK-NEXT:    ret void
 ;
   %val = load ptr, ptr %p, align 8, !nonnull !{}
   %val.int = ptrtoint ptr %val to i64
   %val2 = load i32, ptr %p, align 8
   call void @use.i64(i64 %val.int)
   call void @use.i32(i32 %val2)
   ret void
 }
 
 define void @load_i64_range_to_i32_range(ptr %p) {
 ; CHECK-LABEL: define void @load_i64_range_to_i32_range
 ; CHECK-SAME: (ptr [[P:%.*]]) {
-; CHECK-NEXT:    [[VAL:%.*]] = load i64, ptr [[P]], align 8, !range [[RNG8:![0-9]+]]
+; CHECK-NEXT:    [[VAL:%.*]] = load i64, ptr [[P]], align 8
 ; CHECK-NEXT:    [[TMP1:%.*]] = trunc i64 [[VAL]] to i32
 ; CHECK-NEXT:    call void @use.i64(i64 [[VAL]])
 ; CHECK-NEXT:    call void @use.i32(i32 [[TMP1]])
 ; CHECK-NEXT:    ret void
 ;
   %val = load i64, ptr %p, align 8, !range !{i64 0, i64 10}
   %val2 = load i32, ptr %p, align 8, !range !{i32 0, i32 10}
   call void @use.i64(i64 %val)

 ; CHECK: [[RNG0]] = !{i32 0, i32 2}
 ; CHECK: [[RNG1]] = !{i32 0, i32 2, i32 3, i32 5}
 ; CHECK: [[RNG2]] = !{i32 0, i32 5}
 ; CHECK: [[RNG3]] = !{i32 -5, i32 -2, i32 1, i32 5}
 ; CHECK: [[RNG4]] = !{i32 10, i32 1}
 ; CHECK: [[RNG5]] = !{i32 3, i32 4, i32 5, i32 2}
 ; CHECK: [[META6:![0-9]+]] = !{}
 ; CHECK: [[META7:![0-9]+]] = !{i64 10}
-; CHECK: [[RNG8]] = !{i64 0, i64 10}
 ;.