From 62332e998e5139737b1a35208a3129838a931482 Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Fri, 15 Nov 2024 13:13:04 +0000 Subject: [PATCH 1/3] [MergedLoadStore] Preserve common metadata when sinking stores. When sinking a store, preserve common metadata present on stores on both sides of the diamond. --- .../Transforms/Scalar/MergedLoadStoreMotion.cpp | 10 +++++++++- .../preserve-store-metadata.ll | 14 ++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp index 299239fb70200..2cdca867215c4 100644 --- a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp +++ b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp @@ -255,7 +255,15 @@ void MergedLoadStoreMotion::sinkStoresAndGEPs(BasicBlock *BB, StoreInst *S0, BasicBlock::iterator InsertPt = BB->getFirstInsertionPt(); // Intersect optional metadata. S0->andIRFlags(S1); - S0->dropUnknownNonDebugMetadata(); + + // Keep metadata present on both instructions. + SmallVector> MDs; + S0->getAllMetadataOtherThanDebugLoc(MDs); + SmallVector CommonMDs; + for (const auto &[Kind, MD] : MDs) + if (S1->getMetadata(Kind) == MD) + CommonMDs.push_back(Kind); + S0->dropUnknownNonDebugMetadata(CommonMDs); S0->applyMergedLocation(S0->getDebugLoc(), S1->getDebugLoc()); S0->mergeDIAssignID(S1); diff --git a/llvm/test/Transforms/MergedLoadStoreMotion/preserve-store-metadata.ll b/llvm/test/Transforms/MergedLoadStoreMotion/preserve-store-metadata.ll index f733003e90729..03cb40fa4a609 100644 --- a/llvm/test/Transforms/MergedLoadStoreMotion/preserve-store-metadata.ll +++ b/llvm/test/Transforms/MergedLoadStoreMotion/preserve-store-metadata.ll @@ -19,7 +19,7 @@ define void @perserve_common_metadata(i1 %c, ptr %dst, ptr %min) { ; CHECK-NEXT: br label %[[RETURN]] ; CHECK: [[RETURN]]: ; CHECK-NEXT: [[DOTSINK:%.*]] = phi ptr [ [[DST]], %[[THEN]] ], [ null, %[[ELSE]] ] -; CHECK-NEXT: store ptr [[DOTSINK]], ptr [[GEP_DST_16]], align 8 +; CHECK-NEXT: store ptr [[DOTSINK]], ptr [[GEP_DST_16]], align 8, !tbaa [[TBAA4:![0-9]+]], !alias.scope [[META6:![0-9]+]], !noalias [[META6]], !llvm.access.group [[ACC_GRP9:![0-9]+]] ; CHECK-NEXT: ret void ; entry: @@ -48,7 +48,7 @@ define void @clear_different_metadata(i1 %c, ptr %dst, ptr %min) { ; CHECK-NEXT: [[GEP_DST_16:%.*]] = getelementptr inbounds nuw i8, ptr [[DST]], i64 16 ; CHECK-NEXT: br i1 [[C]], label %[[THEN:.*]], label %[[ELSE:.*]] ; CHECK: [[THEN]]: -; CHECK-NEXT: store ptr [[DST]], ptr [[MIN]], align 8, !tbaa [[TBAA4:![0-9]+]] +; CHECK-NEXT: store ptr [[DST]], ptr [[MIN]], align 8, !tbaa [[TBAA10:![0-9]+]] ; CHECK-NEXT: br label %[[RETURN:.*]] ; CHECK: [[ELSE]]: ; CHECK-NEXT: [[GEP_DST_24:%.*]] = getelementptr inbounds nuw i8, ptr [[DST]], i64 24 @@ -99,6 +99,12 @@ return: ; CHECK: [[META2]] = !{!"omnipotent char", [[META3:![0-9]+]], i64 0} ; CHECK: [[META3]] = !{!"Simple C++ TBAA"} ; CHECK: [[TBAA4]] = !{[[META5:![0-9]+]], [[META5]], i64 0, i64 0} -; CHECK: [[META5]] = !{!"p2 _Foo", [[META6:![0-9]+]]} -; CHECK: [[META6]] = !{!"any pointer", [[META2]], i64 0} +; CHECK: [[META5]] = !{!"long", [[META2]]} +; CHECK: [[META6]] = !{[[META7:![0-9]+]]} +; CHECK: [[META7]] = distinct !{[[META7]], [[META8:![0-9]+]]} +; CHECK: [[META8]] = distinct !{[[META8]]} +; CHECK: [[ACC_GRP9]] = distinct !{} +; CHECK: [[TBAA10]] = !{[[META11:![0-9]+]], [[META11]], i64 0, i64 0} +; CHECK: [[META11]] = !{!"p2 _Foo", [[META12:![0-9]+]]} +; CHECK: [[META12]] = !{!"any pointer", [[META2]], i64 0} ;. From b68c58090b754abdf85cdd35909ab6b3d559a078 Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Fri, 15 Nov 2024 13:38:04 +0000 Subject: [PATCH 2/3] !fixup use combineMetadataForCSE. --- llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp index 2cdca867215c4..cc67a455672be 100644 --- a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp +++ b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp @@ -84,6 +84,7 @@ #include "llvm/Support/raw_ostream.h" #include "llvm/Transforms/Scalar.h" #include "llvm/Transforms/Utils/BasicBlockUtils.h" +#include "llvm/Transforms/Utils/Local.h" using namespace llvm; @@ -256,14 +257,7 @@ void MergedLoadStoreMotion::sinkStoresAndGEPs(BasicBlock *BB, StoreInst *S0, // Intersect optional metadata. S0->andIRFlags(S1); - // Keep metadata present on both instructions. - SmallVector> MDs; - S0->getAllMetadataOtherThanDebugLoc(MDs); - SmallVector CommonMDs; - for (const auto &[Kind, MD] : MDs) - if (S1->getMetadata(Kind) == MD) - CommonMDs.push_back(Kind); - S0->dropUnknownNonDebugMetadata(CommonMDs); + combineMetadataForCSE(S0, S1, true); S0->applyMergedLocation(S0->getDebugLoc(), S1->getDebugLoc()); S0->mergeDIAssignID(S1); From 5653a5c24af7c66937847ca393d03f911a1823d1 Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Fri, 15 Nov 2024 13:53:40 +0000 Subject: [PATCH 3/3] !fixup remove FIXME --- .../Transforms/MergedLoadStoreMotion/preserve-store-metadata.ll | 1 - 1 file changed, 1 deletion(-) diff --git a/llvm/test/Transforms/MergedLoadStoreMotion/preserve-store-metadata.ll b/llvm/test/Transforms/MergedLoadStoreMotion/preserve-store-metadata.ll index 03cb40fa4a609..33e37c97b7a0e 100644 --- a/llvm/test/Transforms/MergedLoadStoreMotion/preserve-store-metadata.ll +++ b/llvm/test/Transforms/MergedLoadStoreMotion/preserve-store-metadata.ll @@ -3,7 +3,6 @@ target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-n32:64-S128-Fn32" -; FIXME: Can preserve common metadata on the sunk store. define void @perserve_common_metadata(i1 %c, ptr %dst, ptr %min) { ; CHECK-LABEL: define void @perserve_common_metadata( ; CHECK-SAME: i1 [[C:%.*]], ptr [[DST:%.*]], ptr [[MIN:%.*]]) {