Always produce canonical form when combining metadata

Author: nikic

llvm/include/llvm/IR/Instructions.h

@@ -393,9 +393,6 @@ class StoreInst : public Instruction {
     return getPointerOperandType()->getPointerAddressSpace();
   }
 
-  /// Get capturing behavior of the value operand, based on !captures metadata.
-  CaptureComponents getCaptureComponents() const;
-
   // Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const Instruction *I) {
     return I->getOpcode() == Instruction::Store;

llvm/include/llvm/IR/Metadata.h

@@ -41,6 +41,7 @@
 
 namespace llvm {
 
+enum class CaptureComponents : uint8_t;
 class Module;
 class ModuleSlotTracker;
 class raw_ostream;
@@ -1480,6 +1481,13 @@ class MDNode : public Metadata {
   LLVM_ABI static MDNode *getMergedCallsiteMetadata(MDNode *A, MDNode *B);
   LLVM_ABI static MDNode *getMergedCalleeTypeMetadata(const MDNode *A,
                                                       const MDNode *B);
+
+  /// Convert !captures metadata to CaptureComponents. MD may be nullptr.
+  LLVM_ABI static CaptureComponents toCaptureComponents(const MDNode *MD);
+  /// Convert CaptureComponents to !captures metadata. The return value may be
+  /// nullptr.
+  LLVM_ABI static MDNode *fromCaptureComponents(LLVMContext &Ctx,
+                                                CaptureComponents CC);
 };
 
 /// Tuple of metadata.

llvm/lib/Analysis/CaptureTracking.cpp

@@ -321,7 +321,8 @@ UseCaptureInfo llvm::DetermineUseCaptureKind(const Use &U, const Value *Base) {
   case Instruction::Store:
     // Stored the pointer - conservatively assume it may be captured.
     if (U.getOperandNo() == 0)
-      return cast<StoreInst>(I)->getCaptureComponents();
+      return MDNode::toCaptureComponents(
+          I->getMetadata(LLVMContext::MD_captures));
 
     // Volatile stores make the address observable.
     if (cast<StoreInst>(I)->isVolatile())

llvm/lib/IR/Instructions.cpp

@@ -1390,24 +1390,6 @@ StoreInst::StoreInst(Value *val, Value *addr, bool isVolatile, Align Align,
   AssertOK();
 }
 
-CaptureComponents StoreInst::getCaptureComponents() const {
-  const MDNode *MD = getMetadata(LLVMContext::MD_captures);
-  if (!MD)
-    return CaptureComponents::All;
-
-  CaptureComponents CC = CaptureComponents::None;
-  for (Metadata *Op : MD->operands()) {
-    CaptureComponents Component =
-        StringSwitch<CaptureComponents>(cast<MDString>(Op)->getString())
-            .Case("address", CaptureComponents::Address)
-            .Case("address_is_null", CaptureComponents::AddressIsNull)
-            .Case("provenance", CaptureComponents::Provenance)
-            .Case("read_provenance", CaptureComponents::ReadProvenance);
-    CC |= Component;
-  }
-  return CC;
-}
-
 //===----------------------------------------------------------------------===//
 //                       AtomicCmpXchgInst Implementation
 //===----------------------------------------------------------------------===//

llvm/lib/IR/Metadata.cpp

@@ -48,6 +48,7 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
+#include "llvm/Support/ModRef.h"
 #include <cassert>
 #include <cstddef>
 #include <cstdint>
@@ -1435,6 +1436,40 @@ MDNode *MDNode::getMostGenericAlignmentOrDereferenceable(MDNode *A, MDNode *B) {
   return B;
 }
 
+CaptureComponents MDNode::toCaptureComponents(const MDNode *MD) {
+  if (!MD)
+    return CaptureComponents::All;
+
+  CaptureComponents CC = CaptureComponents::None;
+  for (Metadata *Op : MD->operands()) {
+    CaptureComponents Component =
+        StringSwitch<CaptureComponents>(cast<MDString>(Op)->getString())
+            .Case("address", CaptureComponents::Address)
+            .Case("address_is_null", CaptureComponents::AddressIsNull)
+            .Case("provenance", CaptureComponents::Provenance)
+            .Case("read_provenance", CaptureComponents::ReadProvenance);
+    CC |= Component;
+  }
+  return CC;
+}
+
+MDNode *MDNode::fromCaptureComponents(LLVMContext &Ctx, CaptureComponents CC) {
+  assert(!capturesNothing(CC) && "Can't encode captures(none)");
+  if (capturesAll(CC))
+    return nullptr;
+
+  SmallVector<Metadata *> Components;
+  if (capturesAddressIsNullOnly(CC))
+    Components.push_back(MDString::get(Ctx, "address_is_null"));
+  else if (capturesAddress(CC))
+    Components.push_back(MDString::get(Ctx, "address"));
+  if (capturesReadProvenanceOnly(CC))
+    Components.push_back(MDString::get(Ctx, "read_provenance"));
+  else if (capturesFullProvenance(CC))
+    Components.push_back(MDString::get(Ctx, "provenance"));
+  return MDNode::get(Ctx, Components);
+}
+
 //===----------------------------------------------------------------------===//
 // NamedMDNode implementation.
 //

llvm/lib/Transforms/Utils/Local.cpp

@@ -3026,7 +3026,10 @@ static void combineMetadata(Instruction *K, const Instruction *J,
         K->setMetadata(Kind, JMD);
         break;
       case LLVMContext::MD_captures:
-        K->setMetadata(Kind, JMD ? MDNode::concatenate(JMD, KMD) : nullptr);
+        K->setMetadata(
+            Kind, MDNode::fromCaptureComponents(
+                      K->getContext(), MDNode::toCaptureComponents(JMD) |
+                                           MDNode::toCaptureComponents(KMD)));
         break;
       }
   }

llvm/test/Transforms/SimplifyCFG/hoist-with-metadata.ll

@@ -469,7 +469,7 @@ out:
 define void @hoist_captures_overlap(i1 %c, ptr %x, ptr %y) {
 ; CHECK-LABEL: @hoist_captures_overlap(
 ; CHECK-NEXT:  if:
-; CHECK-NEXT:    store ptr [[X:%.*]], ptr [[Y:%.*]], align 8, !captures [[META11:![0-9]+]]
+; CHECK-NEXT:    store ptr [[X:%.*]], ptr [[Y:%.*]], align 8, !captures [[META10]]
 ; CHECK-NEXT:    ret void
 ;
 if:
@@ -487,12 +487,52 @@ out:
   ret void
 }
 
-; We could also omit the attribute in this case, as it provides no additional
-; information.
+define void @hoist_captures_subsume1(i1 %c, ptr %x, ptr %y) {
+; CHECK-LABEL: @hoist_captures_subsume1(
+; CHECK-NEXT:  if:
+; CHECK-NEXT:    store ptr [[X:%.*]], ptr [[Y:%.*]], align 8, !captures [[META9]]
+; CHECK-NEXT:    ret void
+;
+if:
+  br i1 %c, label %then, label %else
+
+then:
+  store ptr %x, ptr %y, !captures !{!"address_is_null"}
+  br label %out
+
+else:
+  store ptr %x, ptr %y, !captures !{!"address"}
+  br label %out
+
+out:
+  ret void
+}
+
+define void @hoist_captures_subsume2(i1 %c, ptr %x, ptr %y) {
+; CHECK-LABEL: @hoist_captures_subsume2(
+; CHECK-NEXT:  if:
+; CHECK-NEXT:    store ptr [[X:%.*]], ptr [[Y:%.*]], align 8, !captures [[META11:![0-9]+]]
+; CHECK-NEXT:    ret void
+;
+if:
+  br i1 %c, label %then, label %else
+
+then:
+  store ptr %x, ptr %y, !captures !{!"provenance"}
+  br label %out
+
+else:
+  store ptr %x, ptr %y, !captures !{!"read_provenance"}
+  br label %out
+
+out:
+  ret void
+}
+
 define void @hoist_captures_full_set(i1 %c, ptr %x, ptr %y) {
 ; CHECK-LABEL: @hoist_captures_full_set(
 ; CHECK-NEXT:  if:
-; CHECK-NEXT:    store ptr [[X:%.*]], ptr [[Y:%.*]], align 8, !captures [[META12:![0-9]+]]
+; CHECK-NEXT:    store ptr [[X:%.*]], ptr [[Y:%.*]], align 8
 ; CHECK-NEXT:    ret void
 ;
 if:
@@ -574,7 +614,6 @@ out:
 ; CHECK: [[META7]] = !{i32 5, i32 6}
 ; CHECK: [[META8]] = !{i32 4, i32 5}
 ; CHECK: [[META9]] = !{!"address"}
-; CHECK: [[META10]] = !{!"read_provenance", !"address"}
-; CHECK: [[META11]] = !{!"address", !"read_provenance"}
-; CHECK: [[META12]] = !{!"provenance", !"address"}
+; CHECK: [[META10]] = !{!"address", !"read_provenance"}
+; CHECK: [[META11]] = !{!"provenance"}
 ;.