Preserve load alignment and dereferenceable metadata during some transformations

[apilipenko]

Summary

Preserve new !align, !dereferenceable, !dereferenceable_or_null load metadata everywhere we preserve !nonnull metadata.

[reames]

Test cases? Ideally, these would be split into separate patches. Given the size, I won't insist.

[apilipenko]

Added tests, implemented correct combining of dereferenceable metadata.

[hfinkel]

lib/IR/Metadata.cpp

967

It seems silly to copy-and-paste the same function twice with a different name. How about just naming it getMostGenericAlignmentOrDereferenceable?

[apilipenko]

Address Hal's comment.

[hfinkel]

One comment below, otherwise, this LGTM.

lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

376

Move these up to combine with nonnull handling.

[apilipenko]

lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

376

nonnull code also contains logic for the case when new type is an integer. These can be combined using fall through case, but I don't think that it helps readability.

include/llvm/IR/Metadata.h

#include "llvm/IR/Metadata.def"
 
   /// \brief Methods for metadata merging.
   static MDNode *concatenate(MDNode *A, MDNode *B);
   static MDNode *intersect(MDNode *A, MDNode *B);
   static MDNode *getMostGenericTBAA(MDNode *A, MDNode *B);
   static MDNode *getMostGenericFPMath(MDNode *A, MDNode *B);
   static MDNode *getMostGenericRange(MDNode *A, MDNode *B);
   static MDNode *getMostGenericAliasScope(MDNode *A, MDNode *B);
+  static MDNode *getMostGenericAlignment(MDNode *A, MDNode *B);
+  static MDNode *getMostGenericDereferenceable(MDNode *A, MDNode *B);
+
 };
 
 /// \brief Tuple of metadata.
 ///
 /// This is the simple \a MDNode arbitrary tuple.  Nodes are uniqued by
 /// default based on their operands.
 class MDTuple : public MDNode {
   friend class LLVMContextImpl;

lib/IR/Metadata.cpp

MDNode *MDNode::getMostGenericRange(MDNode *A, MDNode *B) {
 
   SmallVector<Metadata *, 4> MDs;
   MDs.reserve(EndPoints.size());
   for (auto *I : EndPoints)
     MDs.push_back(ConstantAsMetadata::get(I));
   return MDNode::get(A->getContext(), MDs);
 }
 
+MDNode *MDNode::getMostGenericAlignment(MDNode *A, MDNode *B) {
+  if (!A || !B)
+    return nullptr;
+
+  ConstantInt *AVal = mdconst::extract<ConstantInt>(A->getOperand(0));
+  ConstantInt *BVal = mdconst::extract<ConstantInt>(B->getOperand(0));
+  if (AVal->getZExtValue() < BVal->getZExtValue())
+    return A;
+  return B;
+}
+
+MDNode *MDNode::getMostGenericDereferenceable(MDNode *A, MDNode *B) {
+  if (!A || !B)
+    return nullptr;
+
+  ConstantInt *AVal = mdconst::extract<ConstantInt>(A->getOperand(0));
+  ConstantInt *BVal = mdconst::extract<ConstantInt>(B->getOperand(0));
+  if (AVal->getZExtValue() < BVal->getZExtValue())
+    return A;
+  return B;
+}
+
 //===----------------------------------------------------------------------===//
 // NamedMDNode implementation.
 //
 
 static SmallVector<TrackingMDRef, 4> &getNMDOps(void *Operands) {
   return *(SmallVector<TrackingMDRef, 4> *)Operands;
 }
 

lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

case LLVMContext::MD_nonnull:
       }
       break;
 
     case LLVMContext::MD_range:
       // FIXME: It would be nice to propagate this in some way, but the type
       // conversions make it hard. If the new type is a pointer, we could
       // translate it to !nonnull metadata.
       break;
+    case LLVMContext::MD_align:
+    case LLVMContext::MD_dereferenceable:
+    case LLVMContext::MD_dereferenceable_or_null:
+      // These only directly apply if the new type is also a pointer.
+      if (NewTy->isPointerTy()) {
+        NewLoad->setMetadata(ID, N);
+        break;
+      }
     }
   }
   return NewLoad;
 }
 
 /// \brief Combine a store to a new type.
 ///
 /// Returns the newly created store instruction.
for (const auto &MDPair : MD) {
     case LLVMContext::MD_mem_parallel_loop_access:
       // All of these directly apply.
       NewStore->setMetadata(ID, N);
       break;
 
     case LLVMContext::MD_invariant_load:
     case LLVMContext::MD_nonnull:
     case LLVMContext::MD_range:
+    case LLVMContext::MD_align:
+    case LLVMContext::MD_dereferenceable:
+    case LLVMContext::MD_dereferenceable_or_null:
       // These don't apply for stores.
       break;
     }
   }
 
   return NewStore;
 }
 
Instruction *InstCombiner::visitLoadInst(LoadInst &LI) {
   // separated by a few arithmetic operations.
   BasicBlock::iterator BBI(LI);
   AAMDNodes AATags;
   if (Value *AvailableVal =
       FindAvailableLoadedValue(Op, LI.getParent(), BBI,
                                DefMaxInstsToScan, AA, &AATags)) {
     if (LoadInst *NLI = dyn_cast<LoadInst>(AvailableVal)) {
       unsigned KnownIDs[] = {
-          LLVMContext::MD_tbaa,           LLVMContext::MD_alias_scope,
-          LLVMContext::MD_noalias,        LLVMContext::MD_range,
-          LLVMContext::MD_invariant_load, LLVMContext::MD_nonnull,
-          LLVMContext::MD_invariant_group};
+          LLVMContext::MD_tbaa,            LLVMContext::MD_alias_scope,
+          LLVMContext::MD_noalias,         LLVMContext::MD_range,
+          LLVMContext::MD_invariant_load,  LLVMContext::MD_nonnull,
+          LLVMContext::MD_invariant_group, LLVMContext::MD_align,
+          LLVMContext::MD_dereferenceable,
+          LLVMContext::MD_dereferenceable_or_null};
       combineMetadata(NLI, &LI, KnownIDs);
     };
 
     return ReplaceInstUsesWith(
         LI, Builder->CreateBitOrPointerCast(AvailableVal, LI.getType(),
                                             LI.getName() + ".cast"));
   }
 

lib/Transforms/InstCombine/InstCombinePHI.cpp

Instruction *InstCombiner::FoldPHIArgLoadIntoPHI(PHINode &PN) {
   LoadInst *NewLI = new LoadInst(NewPN, "", isVolatile, LoadAlignment);
 
   unsigned KnownIDs[] = {
     LLVMContext::MD_tbaa,
     LLVMContext::MD_range,
     LLVMContext::MD_invariant_load,
     LLVMContext::MD_alias_scope,
     LLVMContext::MD_noalias,
-    LLVMContext::MD_nonnull
+    LLVMContext::MD_nonnull,
+    LLVMContext::MD_align,
+    LLVMContext::MD_dereferenceable,
+    LLVMContext::MD_dereferenceable_or_null,
   };
 
   for (unsigned ID : KnownIDs)
     NewLI->setMetadata(ID, FirstLI->getMetadata(ID));
 
   // Add all operands to the new PHI and combine TBAA metadata.
   for (unsigned i = 1, e = PN.getNumIncomingValues(); i != e; ++i) {
     LoadInst *LI = cast<LoadInst>(PN.getIncomingValue(i));

lib/Transforms/Utils/Local.cpp

switch (Kind) {
         break;
       case LLVMContext::MD_nonnull:
         // Only set the !nonnull if it is present in both instructions.
         K->setMetadata(Kind, JMD);
         break;
       case LLVMContext::MD_invariant_group:
         // Preserve !invariant.group in K.
         break;
+      case LLVMContext::MD_align:
+        K->setMetadata(Kind, MDNode::getMostGenericAlignment(JMD, KMD));
+        break;
+      case LLVMContext::MD_dereferenceable:
+      case LLVMContext::MD_dereferenceable_or_null:
+        K->setMetadata(Kind, MDNode::getMostGenericDereferenceable(JMD, KMD));
+        break;
     }
   }
   // Set !invariant.group from J if J has it. If both instructions have it
   // then we will just pick it from J - even when they are different.
   // Also make sure that K is load or store - f.e. combining bitcast with load
   // could produce bitcast with invariant.group metadata, which is invalid.
   // FIXME: we should try to preserve both invariant.group md if they are
   // different, but right now instruction can only have one invariant.group.

lib/Transforms/Utils/SimplifyCFG.cpp

do {
     // the now redundant second instruction.
     BIParent->getInstList().splice(BI->getIterator(), BB1->getInstList(), I1);
     if (!I2->use_empty())
       I2->replaceAllUsesWith(I1);
     I1->intersectOptionalDataWith(I2);
     unsigned KnownIDs[] = {
         LLVMContext::MD_tbaa,    LLVMContext::MD_range,
         LLVMContext::MD_fpmath,  LLVMContext::MD_invariant_load,
-        LLVMContext::MD_nonnull, LLVMContext::MD_invariant_group};
+        LLVMContext::MD_nonnull, LLVMContext::MD_invariant_group,
+        LLVMContext::MD_align,   LLVMContext::MD_dereferenceable,
+        LLVMContext::MD_dereferenceable_or_null};
     combineMetadata(I1, I2, KnownIDs);
     I2->eraseFromParent();
     Changed = true;
 
     I1 = &*BB1_Itr++;
     I2 = &*BB2_Itr++;
     // Skip debug info if it is not identical.
     DbgInfoIntrinsic *DBI1 = dyn_cast<DbgInfoIntrinsic>(I1);

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

+; RUN: opt -instcombine -S < %s | FileCheck %s
+
+target datalayout = "e-m:e-p:64:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+; CHECK-LABEL: @test_load_load_combine_metadata(
+; Check that align metadata is combined
+; CHECK: load i32*, i32** %0
+; CHECK-SAME: !align ![[ALIGN:[0-9]+]]
+define void @test_load_load_combine_metadata(i32**, i32**, i32**) {
+  %a = load i32*, i32** %0, !align !0
+  %b = load i32*, i32** %0, !align !1
+  store i32 0, i32* %a
+  store i32 0, i32* %b
+  ret void
+}
+
+; CHECK: ![[ALIGN]] = !{i64 4}
+
+!0 = !{i64 4}
+!1 = !{i64 8}
 No newline at end of file

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

+; RUN: opt -instcombine -S < %s | FileCheck %s
+
+target datalayout = "e-m:e-p:64:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+; CHECK-LABEL: @test_load_load_combine_metadata(
+; Check that dereferenceable metadata is combined
+; CHECK: load i32*, i32** %0
+; CHECK-SAME: !dereferenceable ![[DEREF:[0-9]+]]
+define void @test_load_load_combine_metadata(i32**, i32**, i32**) {
+  %a = load i32*, i32** %0, !dereferenceable !0
+  %b = load i32*, i32** %0, !dereferenceable !1
+  store i32 0, i32* %a
+  store i32 0, i32* %b
+  ret void
+}
+
+; CHECK: ![[DEREF]] = !{i64 4}
+
+!0 = !{i64 4}
+!1 = !{i64 8}
 No newline at end of file

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

+; RUN: opt -instcombine -S < %s | FileCheck %s
+
+target datalayout = "e-m:e-p:64:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+; CHECK-LABEL: @test_load_load_combine_metadata(
+; Check that dereferenceable_or_null metadata is combined
+; CHECK: load i32*, i32** %0
+; CHECK-SAME: !dereferenceable_or_null ![[DEREF:[0-9]+]]
+define void @test_load_load_combine_metadata(i32**, i32**, i32**) {
+  %a = load i32*, i32** %0, !dereferenceable_or_null !0
+  %b = load i32*, i32** %0, !dereferenceable_or_null !1
+  store i32 0, i32* %a
+  store i32 0, i32* %b
+  ret void
+}
+
+; CHECK: ![[DEREF]] = !{i64 4}
+
+!0 = !{i64 4}
+!1 = !{i64 8}

test/Transforms/InstCombine/loadstore-metadata.ll

 ; Ensure (cast (load (...))) -> (load (cast (...))) drops range metadata. It
 ; would be nice to preserve or update it somehow but this is hard when moving
 ; between types.
 ; CHECK-LABEL: @test_load_cast_combine_range(
 ; CHECK: load float, float* %{{.*}}
 ; CHECK-NOT: !range
 ; CHECK: ret float
 entry:
-  %l = load i32, i32* %ptr, !range !5
+  %l = load i32, i32* %ptr, !range !6
   %c = bitcast i32 %l to float
   ret float %c
 }
 
 define i32 @test_load_cast_combine_invariant(float* %ptr) {
 ; Ensure (cast (load (...))) -> (load (cast (...))) preserves invariant metadata.
 ; CHECK-LABEL: @test_load_cast_combine_invariant(
 ; CHECK: load i32, i32* %{{.*}}, !invariant.load !3

 ; CHECK-LABEL: @test_load_cast_combine_nontemporal(
 ; CHECK: load i32, i32* %{{.*}}, !nontemporal !4
 entry:
   %l = load float, float* %ptr, !nontemporal !4
   %c = bitcast float %l to i32
   ret i32 %c
 }
 
+define i8* @test_load_cast_combine_align(i32** %ptr) {
+; Ensure (cast (load (...))) -> (load (cast (...))) preserves align
+; metadata.
+; CHECK-LABEL: @test_load_cast_combine_align(
+; CHECK: load i8*, i8** %{{.*}}, !align !5
+entry:
+  %l = load i32*, i32** %ptr, !align !5
+  %c = bitcast i32* %l to i8*
+  ret i8* %c
+}
+
+define i8* @test_load_cast_combine_deref(i32** %ptr) {
+; Ensure (cast (load (...))) -> (load (cast (...))) preserves dereferenceable
+; metadata.
+; CHECK-LABEL: @test_load_cast_combine_deref(
+; CHECK: load i8*, i8** %{{.*}}, !dereferenceable !5
+entry:
+  %l = load i32*, i32** %ptr, !dereferenceable !5
+  %c = bitcast i32* %l to i8*
+  ret i8* %c
+}
+
+define i8* @test_load_cast_combine_deref_or_null(i32** %ptr) {
+; Ensure (cast (load (...))) -> (load (cast (...))) preserves
+; dereferenceable_or_null metadata.
+; CHECK-LABEL: @test_load_cast_combine_deref_or_null(
+; CHECK: load i8*, i8** %{{.*}}, !dereferenceable_or_null !5
+entry:
+  %l = load i32*, i32** %ptr, !dereferenceable_or_null !5
+  %c = bitcast i32* %l to i8*
+  ret i8* %c
+}
+
 define void @test_load_cast_combine_loop(float* %src, i32* %dst, i32 %n) {
 ; Ensure (cast (load (...))) -> (load (cast (...))) preserves loop access
 ; metadata.
 ; CHECK-LABEL: @test_load_cast_combine_loop(
 ; CHECK: load i32, i32* %{{.*}}, !llvm.mem.parallel_loop_access !1
 entry:
   br label %loop
 

 
 ; This is the metadata tuple that we reference above:
 ; CHECK: ![[MD]] = !{i64 1, i64 0}
 !0 = !{ !1, !1, i64 0 }
 !1 = !{ !1 }
 !2 = !{ !2, !1 }
 !3 = !{ }
 !4 = !{ i32 1 }
-!5 = !{ i32 0, i32 42 }
+!5 = !{ i64 8 }
+!6 = !{ i32 0, i32 42 }

test/Transforms/InstCombine/phi-load-metadata-2.ll

+; RUN: opt -instcombine -S < %s | FileCheck %s
+
+declare void @bar()
+declare void @baz()
+
+; Check that dereferenceable metadata is combined
+; CHECK-LABEL: cont:
+; CHECK: load i32*, i32**
+; CHECK-SAME: !dereferenceable ![[DEREF:[0-9]+]]
+define i32* @test_phi_combine_load_metadata(i1 %c, i32** dereferenceable(8) %p1, i32** dereferenceable(8) %p2) {
+  br i1 %c, label %t, label %f
+t:
+  call void @bar()
+  %v1 = load i32*, i32** %p1, align 8, !dereferenceable !0
+  br label %cont
+
+f:
+  call void @baz()
+  %v2 = load i32*, i32** %p2, align 8, !dereferenceable !1
+  br label %cont
+
+cont:
+  %res = phi i32* [ %v1, %t ], [ %v2, %f ]
+  ret i32* %res
+}
+
+; CHECK: ![[DEREF]] = !{i64 8}
+
+!0 = !{i64 8}
+!1 = !{i64 16}

test/Transforms/InstCombine/phi-load-metadata-3.ll

+; RUN: opt -instcombine -S < %s | FileCheck %s
+
+declare void @bar()
+declare void @baz()
+
+; Check that dereferenceable_or_null metadata is combined
+; CHECK-LABEL: cont:
+; CHECK: load i32*, i32**
+; CHECK-SAME: !dereferenceable_or_null ![[DEREF:[0-9]+]]
+define i32* @test_phi_combine_load_metadata(i1 %c, i32** dereferenceable(8) %p1, i32** dereferenceable(8) %p2) {
+  br i1 %c, label %t, label %f
+t:
+  call void @bar()
+  %v1 = load i32*, i32** %p1, align 8, !dereferenceable_or_null !0
+  br label %cont
+
+f:
+  call void @baz()
+  %v2 = load i32*, i32** %p2, align 8, !dereferenceable_or_null !1
+  br label %cont
+
+cont:
+  %res = phi i32* [ %v1, %t ], [ %v2, %f ]
+  ret i32* %res
+}
+
+; CHECK: ![[DEREF]] = !{i64 8}
+
+!0 = !{i64 8}
+!1 = !{i64 16}

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

+; RUN: opt -instcombine -S < %s | FileCheck %s
+
+declare void @bar()
+declare void @baz()
+
+; Check that align metadata is combined
+; CHECK-LABEL: cont:
+; CHECK: load i32*, i32**
+; CHECK-SAME: !align ![[ALIGN:[0-9]+]]
+define i32* @test_phi_combine_load_metadata(i1 %c, i32** dereferenceable(8) %p1, i32** dereferenceable(8) %p2) {
+  br i1 %c, label %t, label %f
+t:
+  call void @bar()
+  %v1 = load i32*, i32** %p1, align 8, !align !0
+  br label %cont
+
+f:
+  call void @baz()
+  %v2 = load i32*, i32** %p2, align 8, !align !1
+  br label %cont
+
+cont:
+  %res = phi i32* [ %v1, %t ], [ %v2, %f ]
+  ret i32* %res
+}
+
+; CHECK: ![[ALIGN]] = !{i64 8}
+
+!0 = !{i64 8}
+!1 = !{i64 16}

test/Transforms/SimplifyCFG/preserve-load-metadata-2.ll

+; RUN: opt < %s -simplifycfg -S | FileCheck %s
+
+declare void @bar(i32*)
+declare void @baz(i32*)
+
+; CHECK-LABEL: @test_load_combine_metadata(
+; Check that dereferenceable metadata is combined
+; CHECK: load i32*, i32** %p
+; CHECK-SAME: !dereferenceable ![[DEREF:[0-9]+]]
+; CHECK: t:
+; CHECK: f:
+define void @test_load_combine_metadata(i1 %c, i32** %p) {
+  br i1 %c, label %t, label %f
+
+t:
+  %v1 = load i32*, i32** %p, !dereferenceable !0
+  call void @bar(i32* %v1)
+  br label %cont
+
+f:
+  %v2 = load i32*, i32** %p, !dereferenceable !1
+  call void @baz(i32* %v2)
+  br label %cont
+
+cont:
+  ret void
+}
+
+; CHECK: ![[DEREF]] = !{i64 8}
+
+!0 = !{i64 8}
+!1 = !{i64 16}

test/Transforms/SimplifyCFG/preserve-load-metadata-3.ll

+; RUN: opt < %s -simplifycfg -S | FileCheck %s
+
+declare void @bar(i32*)
+declare void @baz(i32*)
+
+; CHECK-LABEL: @test_load_combine_metadata(
+; Check that dereferenceable_or_null metadata is combined
+; CHECK: load i32*, i32** %p
+; CHECK-SAME: !dereferenceable_or_null ![[DEREF:[0-9]+]]
+; CHECK: t:
+; CHECK: f:
+define void @test_load_combine_metadata(i1 %c, i32** %p) {
+  br i1 %c, label %t, label %f
+
+t:
+  %v1 = load i32*, i32** %p, !dereferenceable_or_null !0
+  call void @bar(i32* %v1)
+  br label %cont
+
+f:
+  %v2 = load i32*, i32** %p, !dereferenceable_or_null !1
+  call void @baz(i32* %v2)
+  br label %cont
+
+cont:
+  ret void
+}
+
+; CHECK: ![[DEREF]] = !{i64 8}
+
+!0 = !{i64 8}
+!1 = !{i64 16}

test/Transforms/SimplifyCFG/preserve-load-metadata.ll

+; RUN: opt < %s -simplifycfg -S | FileCheck %s
+
+declare void @bar(i32*)
+declare void @baz(i32*)
+
+; CHECK-LABEL: @test_load_combine_metadata(
+; Check that align metadata is combined
+; CHECK: load i32*, i32** %p
+; CHECK-SAME: !align ![[ALIGN:[0-9]+]]
+; CHECK: t:
+; CHECK: f:
+define void @test_load_combine_metadata(i1 %c, i32** %p) {
+  br i1 %c, label %t, label %f
+
+t:
+  %v1 = load i32*, i32** %p, !align !0
+  call void @bar(i32* %v1)
+  br label %cont
+
+f:
+  %v2 = load i32*, i32** %p, !align !1
+  call void @baz(i32* %v2)
+  br label %cont
+
+cont:
+  ret void
+}
+
+; CHECK: ![[ALIGN]] = !{i64 8}
+
+!0 = !{i64 8}
+!1 = !{i64 16}