[Transforms] Support making mutable versions of new-format TBAA access tags

[kosarev]

Ping.

[kosarev]

Ping.

[hfinkel]

LGTM

lib/IR/MDBuilder.cpp

245

// If Tag is already mutable then return it.

[apilipenko]

llvm/trunk/lib/IR/MDBuilder.cpp

236 ↗

(On Diff #130148)

Typo getOperand(1) instead of getOperand(0). And the test doesn't catch this because in the test Base and Access types are always the same.

[kosarev]

llvm/trunk/lib/IR/MDBuilder.cpp

236 ↗

(On Diff #130148)

Prepared a fix in D42364. Thanks for catching!

include/llvm/IR/MDBuilder.h

public:
   /// \brief Return metadata for a TBAA access tag with the given base type,
   /// final access type, offset of the access relative to the base type, size of
   /// the access and flag indicating whether the accessed object can be
   /// considered immutable for the purposes of the TBAA analysis.
   MDNode *createTBAAAccessTag(MDNode *BaseType, MDNode *AccessType,
                               uint64_t Offset, uint64_t Size,
                               bool IsImmutable = false);
 
+  /// \brief Return mutable version of the given mutable or immutable TBAA
+  /// access tag.
+  MDNode *createMutableTBAAAccessTag(MDNode *Tag);
+
   /// \brief Return metadata containing an irreducible loop header weight.
   MDNode *createIrrLoopHeaderWeight(uint64_t Weight);
 };
 
 } // end namespace llvm
 
 #endif

lib/IR/MDBuilder.cpp

MDNode *MDBuilder::createTBAAAccessTag(MDNode *BaseType, MDNode *AccessType,
   if (IsImmutable) {
     auto *ImmutabilityFlagNode = createConstant(ConstantInt::get(Int64, 1));
     return MDNode::get(Context, {BaseType, AccessType, OffsetNode, SizeNode,
                                  ImmutabilityFlagNode});
   }
   return MDNode::get(Context, {BaseType, AccessType, OffsetNode, SizeNode});
 }
 
+MDNode *MDBuilder::createMutableTBAAAccessTag(MDNode *Tag) {
+  MDNode *BaseType = cast<MDNode>(Tag->getOperand(1));
+  MDNode *AccessType = cast<MDNode>(Tag->getOperand(1));
+  Metadata *OffsetNode = Tag->getOperand(2);
+  uint64_t Offset = mdconst::extract<ConstantInt>(OffsetNode)->getZExtValue();
+
+  bool NewFormat = isa<MDNode>(AccessType->getOperand(0));
+
+  // See if the tag is already mutable.
+  unsigned ImmutabilityFlagOp = NewFormat ? 4 : 3;
+  if (Tag->getNumOperands() <= ImmutabilityFlagOp)
+    return Tag;
+
+  Metadata *ImmutabilityFlagNode = Tag->getOperand(ImmutabilityFlagOp);
+  if (!mdconst::extract<ConstantInt>(ImmutabilityFlagNode)->getValue())
+    return Tag;
+
+  // Otherwise, create another node.
+  if (!NewFormat)
+    return createTBAAStructTagNode(BaseType, AccessType, Offset);
+
+  Metadata *SizeNode = Tag->getOperand(3);
+  uint64_t Size = mdconst::extract<ConstantInt>(SizeNode)->getZExtValue();
+  return createTBAAAccessTag(BaseType, AccessType, Offset, Size);
+}
+
 MDNode *MDBuilder::createIrrLoopHeaderWeight(uint64_t Weight) {
   SmallVector<Metadata *, 2> Vals(2);
   Vals[0] = createString("loop_header_weight");
   Vals[1] = createConstant(ConstantInt::get(Type::getInt64Ty(Context), Weight));
   return MDNode::get(Context, Vals);
 }

lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

for (Instruction &I : instructions(F)) {
     // the optimizer to sink the load of a memory location past a statepoint,
     // which is incorrect.
     if (auto *II = dyn_cast<IntrinsicInst>(&I))
       if (II->getIntrinsicID() == Intrinsic::invariant_start) {
         InvariantStartInstructions.push_back(II);
         continue;
       }
 
-    if (const MDNode *MD = I.getMetadata(LLVMContext::MD_tbaa)) {
-      assert(MD->getNumOperands() < 5 && "unrecognized metadata shape!");
-      bool IsImmutableTBAA =
-          MD->getNumOperands() == 4 &&
-          mdconst::extract<ConstantInt>(MD->getOperand(3))->getValue() == 1;
-
-      if (!IsImmutableTBAA)
-        continue; // no work to do, MD_tbaa is already marked mutable
-
-      MDNode *Base = cast<MDNode>(MD->getOperand(0));
-      MDNode *Access = cast<MDNode>(MD->getOperand(1));
-      uint64_t Offset =
-          mdconst::extract<ConstantInt>(MD->getOperand(2))->getZExtValue();
-
-      MDNode *MutableTBAA =
-          Builder.createTBAAStructTagNode(Base, Access, Offset);
+    if (MDNode *Tag = I.getMetadata(LLVMContext::MD_tbaa)) {
+      MDNode *MutableTBAA = Builder.createMutableTBAAAccessTag(Tag);
       I.setMetadata(LLVMContext::MD_tbaa, MutableTBAA);
     }
 
     stripInvalidMetadataFromInstruction(I);
 
     if (CallSite CS = CallSite(&I)) {
       for (int i = 0, e = CS.arg_size(); i != e; i++)
         if (isa<PointerType>(CS.getArgument(i)->getType()))

test/Transforms/RewriteStatepointsForGC/deref-pointers.ll

 entry:
   %a = call noalias i8 addrspace(1)* @some_function()
   call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
   ret i8 addrspace(1)* %a
 }
 
 define i8 @test_md(i8 addrspace(1)* %ptr) gc "statepoint-example" {
 ; CHECK-LABEL: @test_md(
-; CHECK: %tmp = load i8, i8 addrspace(1)* %ptr, !tbaa !0
+; CHECK: %tmp = load i8, i8 addrspace(1)* %ptr, !tbaa [[TAG_old:!.*]]
 entry:
   %tmp = load i8, i8 addrspace(1)* %ptr, !tbaa !0
   call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
   ret i8 %tmp
 }
 
+; Same as test_md() above, but with new-format TBAA metadata.
+define i8 @test_md_new(i8 addrspace(1)* %ptr) gc "statepoint-example" {
+; CHECK-LABEL: @test_md_new(
+; CHECK: %tmp = load i8, i8 addrspace(1)* %ptr, !tbaa [[TAG_new:!.*]]
+entry:
+  %tmp = load i8, i8 addrspace(1)* %ptr, !tbaa !3
+  call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
+  ret i8 %tmp
+}
+
 define i8 addrspace(1)* @test_decl_only_attribute(i8 addrspace(1)* %ptr) gc "statepoint-example" {
 ; CHECK-LABEL: @test_decl_only_attribute(
 ; No change here, but the prototype of some_function_ret_deref should have changed.
 ; CHECK: call i8 addrspace(1)* @some_function_ret_deref()
 entry:
   %a = call i8 addrspace(1)* @some_function_ret_deref()
   call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
   ret i8 addrspace(1)* %a
 }
 
 define i8 addrspace(1)* @test_decl_only_noalias(i8 addrspace(1)* %ptr) gc "statepoint-example" {
 ; CHECK-LABEL: @test_decl_only_noalias(
 ; No change here, but the prototype of some_function_ret_noalias should have changed.
 ; CHECK: call i8 addrspace(1)* @some_function_ret_noalias()
 entry:
   %a = call i8 addrspace(1)* @some_function_ret_noalias()
   call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
   ret i8 addrspace(1)* %a
 }
 
 define i8 addrspace(1)* @test_callsite_arg_attribute(i8 addrspace(1)* %ptr) gc "statepoint-example" {
 ; CHECK-LABEL: @test_callsite_arg_attribute(
 ; CHECK: call void @some_function_consumer(i8 addrspace(1)* %ptr)
-; CHECK: !0 = !{!1, !1, i64 0}
-; CHECK: !1 = !{!"red", !2}
-; CHECK: !2 = !{!"blue"}
 entry:
   call void @some_function_consumer(i8 addrspace(1)* dereferenceable(4) noalias %ptr)
   call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
   ret i8 addrspace(1)* %ptr
 }
-!0 = !{!1, !1, i64 0, i64 1}
-!1 = !{!"red", !2}
-!2 = !{!"blue"}
+
+!0 = !{!1, !1, i64 0, i64 1}  ; TAG_old
+!1 = !{!"type_old", !2}
+!2 = !{!"root"}
+
+!3 = !{!4, !4, i64 0, i64 1, i64 1}  ; TAG_new
+!4 = !{!2, i64 1, !"type_new"}
+
+; CHECK-DAG: [[ROOT:!.*]] = !{!"root"}
+; CHECK-DAG: [[TYPE_old:!.*]] = !{!"type_old", [[ROOT]]}
+; CHECK-DAG: [[TAG_old]] = !{[[TYPE_old]], [[TYPE_old]], i64 0}
+; CHECK-DAG: [[TYPE_new:!.*]] = !{[[ROOT]], i64 1, !"type_new"}
+; CHECK-DAG: [[TAG_new]] = !{[[TYPE_new]], [[TYPE_new]], i64 0, i64 1}