[llvm] [LICM] Only set AA metadata on hoisted load if it executes. (PR #117204)
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Sat Nov 23 04:21:02 PST 2024
https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/117204
>From 5ee04eaae4d5ef53a239fd07ec11089a5f3f8cf2 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Thu, 21 Nov 2024 18:18:28 +0000
Subject: [PATCH 1/3] [LICM] Only set AA metadata on hoisted load if it
executes.
https://github.com/llvm/llvm-project/pull/116220 clarified that
violations of aliasing metadata are UB.
Only set the AA metadata after hoisting a log, if it is guaranteed to
execute in the original loop.
---
llvm/lib/Transforms/Scalar/LICM.cpp | 8 +++++++-
llvm/test/Transforms/LICM/hoist-metadata.ll | 4 ++--
llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll | 4 +++-
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index 94bfe44a847a37..0d2fb6e81b211b 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -2030,7 +2030,9 @@ bool llvm::promoteLoopAccessesToScalars(
bool DereferenceableInPH = false;
bool StoreIsGuanteedToExecute = false;
+ bool LoadIsGuanteedToExecute = false;
bool FoundLoadToPromote = false;
+
// Goes from Unknown to either Safe or Unsafe, but can't switch between them.
enum {
StoreSafe,
@@ -2088,11 +2090,15 @@ bool llvm::promoteLoopAccessesToScalars(
FoundLoadToPromote = true;
Align InstAlignment = Load->getAlign();
+ bool GuaranteedToExecute =
+ SafetyInfo->isGuaranteedToExecute(*UI, DT, CurLoop);
+ LoadIsGuanteedToExecute |= GuaranteedToExecute;
// Note that proving a load safe to speculate requires proving
// sufficient alignment at the target location. Proving it guaranteed
// to execute does as well. Thus we can increase our guaranteed
// alignment as well.
+
if (!DereferenceableInPH || (InstAlignment > Alignment))
if (isSafeToExecuteUnconditionally(
*Load, DT, TLI, CurLoop, SafetyInfo, ORE,
@@ -2247,7 +2253,7 @@ bool llvm::promoteLoopAccessesToScalars(
PreheaderLoad->setOrdering(AtomicOrdering::Unordered);
PreheaderLoad->setAlignment(Alignment);
PreheaderLoad->setDebugLoc(DebugLoc());
- if (AATags)
+ if (AATags && LoadIsGuanteedToExecute)
PreheaderLoad->setAAMetadata(AATags);
MemoryAccess *PreheaderLoadMemoryAccess = MSSAU.createMemoryAccessInBB(
diff --git a/llvm/test/Transforms/LICM/hoist-metadata.ll b/llvm/test/Transforms/LICM/hoist-metadata.ll
index 60b61944b33ae0..fc66d98725b524 100644
--- a/llvm/test/Transforms/LICM/hoist-metadata.ll
+++ b/llvm/test/Transforms/LICM/hoist-metadata.ll
@@ -77,7 +77,7 @@ define void @noalias_metadata_load_may_not_execute() {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[A:%.*]] = alloca i32, align 16
; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds i32, ptr [[A]]
-; CHECK-NEXT: [[GEP_PROMOTED:%.*]] = load i32, ptr [[GEP]], align 4, !tbaa [[TBAA3:![0-9]+]], !noalias [[META7:![0-9]+]]
+; CHECK-NEXT: [[GEP_PROMOTED:%.*]] = load i32, ptr [[GEP]], align 4
; CHECK-NEXT: br label [[LOOP_HEADER:%.*]]
; CHECK: loop.header:
; CHECK-NEXT: [[ADD1:%.*]] = phi i32 [ [[GEP_PROMOTED]], [[ENTRY:%.*]] ], [ [[ADD:%.*]], [[LOOP_LATCH:%.*]] ]
@@ -92,7 +92,7 @@ define void @noalias_metadata_load_may_not_execute() {
; CHECK-NEXT: br i1 [[CMP]], label [[LOOP_HEADER]], label [[EXIT]]
; CHECK: exit:
; CHECK-NEXT: [[ADD2:%.*]] = phi i32 [ [[ADD]], [[LOOP_LATCH]] ], [ [[ADD1]], [[LOOP_HEADER]] ]
-; CHECK-NEXT: store i32 [[ADD2]], ptr [[GEP]], align 4, !tbaa [[TBAA3]], !noalias [[META7]]
+; CHECK-NEXT: store i32 [[ADD2]], ptr [[GEP]], align 4, !tbaa [[TBAA3:![0-9]+]], !noalias [[META7:![0-9]+]]
; CHECK-NEXT: ret void
;
entry:
diff --git a/llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll b/llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll
index 570f4230c1a90d..61f0eb19a9bd1b 100644
--- a/llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll
+++ b/llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll
@@ -1,6 +1,8 @@
; RUN: opt -passes=licm %s -S | FileCheck %s
-; CHECK: %arrayidx4.promoted = load i32, ptr %arrayidx4, align 4, !tbaa !{{[0-9]+$}}
+; CHECK: %arrayidx4.promoted = load i32, ptr %arrayidx4, align 4
+; CHECK-NOT: !dbg
+; CHECK: br label %for.body
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
>From 63659986109184d9a865741d7b0f45b42e46468a Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Sat, 23 Nov 2024 11:22:08 +0000
Subject: [PATCH 2/3] !fixup address latest comments, thanks!
---
llvm/lib/Transforms/Scalar/LICM.cpp | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index 0d2fb6e81b211b..c2d8cb68d46a38 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -2030,7 +2030,7 @@ bool llvm::promoteLoopAccessesToScalars(
bool DereferenceableInPH = false;
bool StoreIsGuanteedToExecute = false;
- bool LoadIsGuanteedToExecute = false;
+ bool LoadIsGuaranteedToExecute = false;
bool FoundLoadToPromote = false;
// Goes from Unknown to either Safe or Unsafe, but can't switch between them.
@@ -2090,15 +2090,12 @@ bool llvm::promoteLoopAccessesToScalars(
FoundLoadToPromote = true;
Align InstAlignment = Load->getAlign();
- bool GuaranteedToExecute =
- SafetyInfo->isGuaranteedToExecute(*UI, DT, CurLoop);
- LoadIsGuanteedToExecute |= GuaranteedToExecute;
+ LoadIsGuaranteedToExecut |= SafetyInfo->isGuaranteedToExecute(*UI, DT, CurLoop);
// Note that proving a load safe to speculate requires proving
// sufficient alignment at the target location. Proving it guaranteed
// to execute does as well. Thus we can increase our guaranteed
// alignment as well.
-
if (!DereferenceableInPH || (InstAlignment > Alignment))
if (isSafeToExecuteUnconditionally(
*Load, DT, TLI, CurLoop, SafetyInfo, ORE,
@@ -2253,7 +2250,7 @@ bool llvm::promoteLoopAccessesToScalars(
PreheaderLoad->setOrdering(AtomicOrdering::Unordered);
PreheaderLoad->setAlignment(Alignment);
PreheaderLoad->setDebugLoc(DebugLoc());
- if (AATags && LoadIsGuanteedToExecute)
+ if (AATags && LoadIsGuaranteedToExecute)
PreheaderLoad->setAAMetadata(AATags);
MemoryAccess *PreheaderLoadMemoryAccess = MSSAU.createMemoryAccessInBB(
>From bd15339dedd7832b85a856b628b075a33c097aef Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Sat, 23 Nov 2024 12:16:38 +0000
Subject: [PATCH 3/3] !fixup only preserve AA metadata on store if it executes.
---
llvm/lib/Transforms/Scalar/LICM.cpp | 14 +++++++++-----
llvm/test/Transforms/LICM/hoist-metadata.ll | 9 +--------
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index c2d8cb68d46a38..ce4187e4be1b04 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -1826,6 +1826,7 @@ class LoopPromoter : public LoadAndStorePromoter {
ICFLoopSafetyInfo &SafetyInfo;
bool CanInsertStoresInExitBlocks;
ArrayRef<const Instruction *> Uses;
+ bool StoreIsGuanteedToExecute;
// We're about to add a use of V in a loop exit block. Insert an LCSSA phi
// (if legal) if doing so would add an out-of-loop use to an instruction
@@ -1852,13 +1853,15 @@ class LoopPromoter : public LoadAndStorePromoter {
SmallVectorImpl<MemoryAccess *> &MSSAIP, PredIteratorCache &PIC,
MemorySSAUpdater &MSSAU, LoopInfo &li, DebugLoc dl,
Align Alignment, bool UnorderedAtomic, const AAMDNodes &AATags,
- ICFLoopSafetyInfo &SafetyInfo, bool CanInsertStoresInExitBlocks)
+ ICFLoopSafetyInfo &SafetyInfo, bool CanInsertStoresInExitBlocks,
+ bool StoreIsGuanteedToExecute)
: LoadAndStorePromoter(Insts, S), SomePtr(SP), LoopExitBlocks(LEB),
LoopInsertPts(LIP), MSSAInsertPts(MSSAIP), PredCache(PIC), MSSAU(MSSAU),
LI(li), DL(std::move(dl)), Alignment(Alignment),
UnorderedAtomic(UnorderedAtomic), AATags(AATags),
SafetyInfo(SafetyInfo),
- CanInsertStoresInExitBlocks(CanInsertStoresInExitBlocks), Uses(Insts) {}
+ CanInsertStoresInExitBlocks(CanInsertStoresInExitBlocks), Uses(Insts),
+ StoreIsGuanteedToExecute(StoreIsGuanteedToExecute) {}
void insertStoresInLoopExitBlocks() {
// Insert stores after in the loop exit blocks. Each exit block gets a
@@ -1892,7 +1895,7 @@ class LoopPromoter : public LoadAndStorePromoter {
NewSI->setMetadata(LLVMContext::MD_DIAssignID, NewID);
}
- if (AATags)
+ if (AATags && StoreIsGuanteedToExecute)
NewSI->setAAMetadata(AATags);
MemoryAccess *MSSAInsertPoint = MSSAInsertPts[i];
@@ -2090,7 +2093,8 @@ bool llvm::promoteLoopAccessesToScalars(
FoundLoadToPromote = true;
Align InstAlignment = Load->getAlign();
- LoadIsGuaranteedToExecut |= SafetyInfo->isGuaranteedToExecute(*UI, DT, CurLoop);
+ LoadIsGuaranteedToExecute |=
+ SafetyInfo->isGuaranteedToExecute(*UI, DT, CurLoop);
// Note that proving a load safe to speculate requires proving
// sufficient alignment at the target location. Proving it guaranteed
@@ -2237,7 +2241,7 @@ bool llvm::promoteLoopAccessesToScalars(
LoopPromoter Promoter(SomePtr, LoopUses, SSA, ExitBlocks, InsertPts,
MSSAInsertPts, PIC, MSSAU, *LI, DL, Alignment,
SawUnorderedAtomic, AATags, *SafetyInfo,
- StoreSafety == StoreSafe);
+ StoreSafety == StoreSafe, StoreIsGuanteedToExecute);
// Set up the preheader to have a definition of the value. It is the live-out
// value from the preheader that uses in the loop will use.
diff --git a/llvm/test/Transforms/LICM/hoist-metadata.ll b/llvm/test/Transforms/LICM/hoist-metadata.ll
index fc66d98725b524..f25cb966a37fc7 100644
--- a/llvm/test/Transforms/LICM/hoist-metadata.ll
+++ b/llvm/test/Transforms/LICM/hoist-metadata.ll
@@ -92,7 +92,7 @@ define void @noalias_metadata_load_may_not_execute() {
; CHECK-NEXT: br i1 [[CMP]], label [[LOOP_HEADER]], label [[EXIT]]
; CHECK: exit:
; CHECK-NEXT: [[ADD2:%.*]] = phi i32 [ [[ADD]], [[LOOP_LATCH]] ], [ [[ADD1]], [[LOOP_HEADER]] ]
-; CHECK-NEXT: store i32 [[ADD2]], ptr [[GEP]], align 4, !tbaa [[TBAA3:![0-9]+]], !noalias [[META7:![0-9]+]]
+; CHECK-NEXT: store i32 [[ADD2]], ptr [[GEP]], align 4
; CHECK-NEXT: ret void
;
entry:
@@ -132,11 +132,4 @@ exit:
; CHECK: [[RNG0]] = !{i32 0, i32 10}
; CHECK: [[META1]] = !{}
; CHECK: [[META2]] = !{i64 4}
-; CHECK: [[TBAA3]] = !{[[META4:![0-9]+]], [[META4]], i64 0}
-; CHECK: [[META4]] = !{!"short", [[META5:![0-9]+]], i64 0}
-; CHECK: [[META5]] = !{!"omnipotent char", [[META6:![0-9]+]], i64 0}
-; CHECK: [[META6]] = !{!"Simple C/C++ TBAA"}
-; CHECK: [[META7]] = !{[[META8:![0-9]+]]}
-; CHECK: [[META8]] = distinct !{[[META8]], [[META9:![0-9]+]]}
-; CHECK: [[META9]] = distinct !{[[META9]]}
;.
More information about the llvm-commits
mailing list