dexonsmith created this revision. dexonsmith added reviewers: dblaikie, aprantl. Herald added a reviewer: bollu. Herald added subscribers: ributzka, hiraditya, kristof.beyls. Herald added a project: LLVM. dexonsmith requested review of this revision.
`TempMDNode` includes a bunch of machinery for RAUW, and should only be used when necessary. RAUW wasn't being used in any of these cases... it was just a placeholder for a self-reference. Where the real node was using `MDNode::getDistinct`, just replace the temporary argument with `nullptr`. Where the real node was using `MDNode::get`, the `replaceOperandWith` call was "promoting" the node to a distinct one implicitly due to self-reference detection in `MDNode::handleChangedOperand`. The `TempMDNode` was serving a purpose by delaying uniquing, but it's way simpler to just call `MDNode::getDistinct` in the first place. Note that using a self-reference at all in these places is a hold-over from before `distinct` metadata existed. It was an old trick to create distinct nodes. It would be intrusive to change, including bitcode upgrades, etc., and it's harmless so I'm not sure there's much value in removing it from existing schemas. After this commit it still has a tiny memory cost (in the extra metadata operand) but no more overhead in construction. https://reviews.llvm.org/D90079 Files: clang/lib/CodeGen/CGLoopInfo.cpp llvm/lib/Analysis/LoopInfo.cpp llvm/lib/IR/MDBuilder.cpp polly/lib/CodeGen/IRBuilder.cpp
Index: polly/lib/CodeGen/IRBuilder.cpp =================================================================== --- polly/lib/CodeGen/IRBuilder.cpp +++ polly/lib/CodeGen/IRBuilder.cpp @@ -26,24 +26,22 @@ /// /// The MDNode looks like this (if arg0/arg1 are not null): /// -/// '!n = metadata !{metadata !n, arg0, arg1}' +/// '!n = distinct !{!n, arg0, arg1}' /// /// @return The self referencing id metadata node. static MDNode *getID(LLVMContext &Ctx, Metadata *arg0 = nullptr, Metadata *arg1 = nullptr) { MDNode *ID; SmallVector<Metadata *, 3> Args; - // Use a temporary node to safely create a unique pointer for the first arg. - auto TempNode = MDNode::getTemporary(Ctx, None); // Reserve operand 0 for loop id self reference. - Args.push_back(TempNode.get()); + Args.push_back(nullptr); if (arg0) Args.push_back(arg0); if (arg1) Args.push_back(arg1); - ID = MDNode::get(Ctx, Args); + ID = MDNode::getDistinct(Ctx, Args); ID->replaceOperandWith(0, ID); return ID; } Index: llvm/lib/IR/MDBuilder.cpp =================================================================== --- llvm/lib/IR/MDBuilder.cpp +++ llvm/lib/IR/MDBuilder.cpp @@ -151,24 +151,20 @@ } MDNode *MDBuilder::createAnonymousAARoot(StringRef Name, MDNode *Extra) { - // To ensure uniqueness the root node is self-referential. - auto Dummy = MDNode::getTemporary(Context, None); - - SmallVector<Metadata *, 3> Args(1, Dummy.get()); + SmallVector<Metadata *, 3> Args(1, nullptr); if (Extra) Args.push_back(Extra); if (!Name.empty()) Args.push_back(createString(Name)); - MDNode *Root = MDNode::get(Context, Args); + MDNode *Root = MDNode::getDistinct(Context, Args); // At this point we have - // !0 = metadata !{} <- dummy - // !1 = metadata !{metadata !0} <- root - // Replace the dummy operand with the root node itself and delete the dummy. + // !0 = distinct !{null} <- root + // Replace the reserved operand with the root node itself. Root->replaceOperandWith(0, Root); // We now have - // !1 = metadata !{metadata !1} <- self-referential root + // !0 = distinct !{!0} <- root return Root; } Index: llvm/lib/Analysis/LoopInfo.cpp =================================================================== --- llvm/lib/Analysis/LoopInfo.cpp +++ llvm/lib/Analysis/LoopInfo.cpp @@ -1017,8 +1017,7 @@ SmallVector<Metadata *, 4> MDs; // Reserve first location for self reference to the LoopID metadata node. - TempMDTuple TempNode = MDNode::getTemporary(Context, None); - MDs.push_back(TempNode.get()); + MDs.push_back(nullptr); // Remove metadata for the transformation that has been applied or that became // outdated. Index: clang/lib/CodeGen/CGLoopInfo.cpp =================================================================== --- clang/lib/CodeGen/CGLoopInfo.cpp +++ clang/lib/CodeGen/CGLoopInfo.cpp @@ -24,8 +24,7 @@ LoopInfo::createLoopPropertiesMetadata(ArrayRef<Metadata *> LoopProperties) { LLVMContext &Ctx = Header->getContext(); SmallVector<Metadata *, 4> NewLoopProperties; - TempMDTuple TempNode = MDNode::getTemporary(Ctx, None); - NewLoopProperties.push_back(TempNode.get()); + NewLoopProperties.push_back(nullptr); NewLoopProperties.append(LoopProperties.begin(), LoopProperties.end()); MDNode *LoopID = MDNode::getDistinct(Ctx, NewLoopProperties); @@ -58,8 +57,7 @@ } SmallVector<Metadata *, 4> Args; - TempMDTuple TempNode = MDNode::getTemporary(Ctx, None); - Args.push_back(TempNode.get()); + Args.push_back(nullptr); Args.append(LoopProperties.begin(), LoopProperties.end()); if (Attrs.PipelineInitiationInterval > 0) { @@ -113,8 +111,7 @@ FollowupHasTransforms); SmallVector<Metadata *, 4> Args; - TempMDTuple TempNode = MDNode::getTemporary(Ctx, None); - Args.push_back(TempNode.get()); + Args.push_back(nullptr); Args.append(LoopProperties.begin(), LoopProperties.end()); // Setting unroll.count @@ -176,8 +173,7 @@ FollowupHasTransforms); SmallVector<Metadata *, 4> Args; - TempMDTuple TempNode = MDNode::getTemporary(Ctx, None); - Args.push_back(TempNode.get()); + Args.push_back(nullptr); Args.append(LoopProperties.begin(), LoopProperties.end()); // Setting unroll_and_jam.count @@ -250,8 +246,7 @@ FollowupHasTransforms); SmallVector<Metadata *, 4> Args; - TempMDTuple TempNode = MDNode::getTemporary(Ctx, None); - Args.push_back(TempNode.get()); + Args.push_back(nullptr); Args.append(LoopProperties.begin(), LoopProperties.end()); // Setting vectorize.predicate @@ -307,7 +302,7 @@ Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.followup_all"), Followup})); - MDNode *LoopID = MDNode::get(Ctx, Args); + MDNode *LoopID = MDNode::getDistinct(Ctx, Args); LoopID->replaceOperandWith(0, LoopID); HasUserTransforms = true; return LoopID; @@ -344,8 +339,7 @@ createLoopVectorizeMetadata(Attrs, LoopProperties, FollowupHasTransforms); SmallVector<Metadata *, 4> Args; - TempMDTuple TempNode = MDNode::getTemporary(Ctx, None); - Args.push_back(TempNode.get()); + Args.push_back(nullptr); Args.append(LoopProperties.begin(), LoopProperties.end()); Metadata *Vals[] = {MDString::get(Ctx, "llvm.loop.distribute.enable"), @@ -359,7 +353,7 @@ Ctx, {MDString::get(Ctx, "llvm.loop.distribute.followup_all"), Followup})); - MDNode *LoopID = MDNode::get(Ctx, Args); + MDNode *LoopID = MDNode::getDistinct(Ctx, Args); LoopID->replaceOperandWith(0, LoopID); HasUserTransforms = true; return LoopID; @@ -389,8 +383,7 @@ } SmallVector<Metadata *, 4> Args; - TempMDTuple TempNode = MDNode::getTemporary(Ctx, None); - Args.push_back(TempNode.get()); + Args.push_back(nullptr); Args.append(LoopProperties.begin(), LoopProperties.end()); Args.push_back(MDNode::get(Ctx, MDString::get(Ctx, "llvm.loop.unroll.full")));
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits