[licm] don't drop `MD_prof` when dropping #152420

mtrofin · 2025-08-07T00:55:46Z

No description provided.

mtrofin · 2025-08-07T00:56:05Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

llvm/lib/IR/Metadata.cpp

llvm/test/Transforms/LICM/hoist-phi-metadata.ll

llvmbot · 2025-08-11T21:34:36Z

@llvm/pr-subscribers-llvm-transforms

Author: Mircea Trofin (mtrofin)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/152420.diff

4 Files Affected:

(modified) llvm/include/llvm/IR/Instruction.h (+4-3)
(modified) llvm/lib/IR/Instruction.cpp (+5-3)
(modified) llvm/lib/Transforms/Scalar/LICM.cpp (+6-2)
(added) llvm/test/Transforms/LICM/hoist-profdata.ll (+45)

diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 5d25804a684ac..2eb4fd36c5b7d 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -584,9 +584,10 @@ class Instruction : public User,
   dropUBImplyingAttrsAndUnknownMetadata(ArrayRef<unsigned> KnownIDs = {});
 
   /// Drop any attributes or metadata that can cause immediate undefined
-  /// behavior. Retain other attributes/metadata on a best-effort basis.
-  /// This should be used when speculating instructions.
-  LLVM_ABI void dropUBImplyingAttrsAndMetadata();
+  /// behavior. Retain other attributes/metadata on a best-effort basis, as well
+  /// as those passed in `Keep`. This should be used when speculating
+  /// instructions.
+  LLVM_ABI void dropUBImplyingAttrsAndMetadata(ArrayRef<unsigned> Keep = {});
 
   /// Return true if this instruction has UB-implying attributes
   /// that can cause immediate undefined behavior.
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index cf19b97333514..2815ea8498b4a 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -554,13 +554,15 @@ void Instruction::dropUBImplyingAttrsAndUnknownMetadata(
   CB->removeRetAttrs(UBImplyingAttributes);
 }
 
-void Instruction::dropUBImplyingAttrsAndMetadata() {
+void Instruction::dropUBImplyingAttrsAndMetadata(ArrayRef<unsigned> Keep) {
   // !annotation metadata does not impact semantics.
   // !range, !nonnull and !align produce poison, so they are safe to speculate.
   // !noundef and various AA metadata must be dropped, as it generally produces
   // immediate undefined behavior.
-  unsigned KnownIDs[] = {LLVMContext::MD_annotation, LLVMContext::MD_range,
-                         LLVMContext::MD_nonnull, LLVMContext::MD_align};
+  SmallVector<unsigned> KnownIDs(Keep.size() + 4);
+  KnownIDs.append({LLVMContext::MD_annotation, LLVMContext::MD_range,
+                   LLVMContext::MD_nonnull, LLVMContext::MD_align});
+  append_range(KnownIDs, Keep);
   dropUBImplyingAttrsAndUnknownMetadata(KnownIDs);
 }
 
diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index 34adc7745f67c..7a203638a1d78 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -1698,8 +1698,12 @@ static void hoist(Instruction &I, const DominatorTree *DT, const Loop *CurLoop,
       // The check on hasMetadataOtherThanDebugLoc is to prevent us from burning
       // time in isGuaranteedToExecute if we don't actually have anything to
       // drop.  It is a compile time optimization, not required for correctness.
-      !SafetyInfo->isGuaranteedToExecute(I, DT, CurLoop))
-    I.dropUBImplyingAttrsAndMetadata();
+      !SafetyInfo->isGuaranteedToExecute(I, DT, CurLoop)) {
+    if (ProfcheckDisableMetadataFixes)
+      I.dropUBImplyingAttrsAndMetadata();
+    else
+      I.dropUBImplyingAttrsAndMetadata({LLVMContext::MD_prof});
+  }
 
   if (isa<PHINode>(I))
     // Move the new node to the end of the phi list in the destination block.
diff --git a/llvm/test/Transforms/LICM/hoist-profdata.ll b/llvm/test/Transforms/LICM/hoist-profdata.ll
new file mode 100644
index 0000000000000..18fa1b9f92e8a
--- /dev/null
+++ b/llvm/test/Transforms/LICM/hoist-profdata.ll
@@ -0,0 +1,45 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals --version 2
+; Test that hoisting conditional branches copies the debug and profiling info
+; metadata from the branch being hoisted.
+; RUN: opt -S -passes=licm %s -o - | FileCheck %s
+
+declare i32 @foo()
+
+; to_hoist should get hoisted, and that should not result
+; in a loss of profiling info
+define i32 @hoist_select(i1 %cond, i32 %a, i32 %b) nounwind {
+; CHECK-LABEL: define i32 @hoist_select
+; CHECK-SAME: (i1 [[COND:%.*]], i32 [[A:%.*]], i32 [[B:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TO_HOIST:%.*]] = select i1 [[COND]], i32 [[A]], i32 [[B]], !prof [[PROF0:![0-9]+]]
+; CHECK-NEXT:    br label [[L0:%.*]]
+; CHECK:       L0:
+; CHECK-NEXT:    [[G:%.*]] = call i32 @foo()
+; CHECK-NEXT:    [[SUM:%.*]] = add i32 [[G]], [[TO_HOIST]]
+; CHECK-NEXT:    [[C:%.*]] = icmp eq i32 [[SUM]], 0
+; CHECK-NEXT:    br i1 [[C]], label [[L0]], label [[EXIT:%.*]], !prof [[PROF1:![0-9]+]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[SUM_LCSSA:%.*]] = phi i32 [ [[SUM]], [[L0]] ]
+; CHECK-NEXT:    ret i32 [[SUM_LCSSA]]
+;
+entry:
+  br label %L0
+L0:
+  %g = call i32 @foo()
+  %to_hoist = select i1 %cond, i32 %a, i32 %b, !prof !0
+  %sum = add i32 %g, %to_hoist
+  %c = icmp eq i32 %sum, 0
+  br i1 %c, label %L0, label %exit, !prof !1
+
+exit:
+  ret i32 %sum
+}
+
+!0 = !{!"branch_weights", i32 2, i32 5}
+!1 = !{!"branch_weights", i32 101, i32 189}
+;.
+; CHECK: attributes #[[ATTR0]] = { nounwind }
+;.
+; CHECK: [[PROF0]] = !{!"branch_weights", i32 2, i32 5}
+; CHECK: [[PROF1]] = !{!"branch_weights", i32 101, i32 189}
+;.

llvmbot · 2025-08-11T21:34:37Z

@llvm/pr-subscribers-llvm-ir

Author: Mircea Trofin (mtrofin)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/152420.diff

4 Files Affected:

(modified) llvm/include/llvm/IR/Instruction.h (+4-3)
(modified) llvm/lib/IR/Instruction.cpp (+5-3)
(modified) llvm/lib/Transforms/Scalar/LICM.cpp (+6-2)
(added) llvm/test/Transforms/LICM/hoist-profdata.ll (+45)

diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 5d25804a684ac..2eb4fd36c5b7d 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -584,9 +584,10 @@ class Instruction : public User,
   dropUBImplyingAttrsAndUnknownMetadata(ArrayRef<unsigned> KnownIDs = {});
 
   /// Drop any attributes or metadata that can cause immediate undefined
-  /// behavior. Retain other attributes/metadata on a best-effort basis.
-  /// This should be used when speculating instructions.
-  LLVM_ABI void dropUBImplyingAttrsAndMetadata();
+  /// behavior. Retain other attributes/metadata on a best-effort basis, as well
+  /// as those passed in `Keep`. This should be used when speculating
+  /// instructions.
+  LLVM_ABI void dropUBImplyingAttrsAndMetadata(ArrayRef<unsigned> Keep = {});
 
   /// Return true if this instruction has UB-implying attributes
   /// that can cause immediate undefined behavior.
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index cf19b97333514..2815ea8498b4a 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -554,13 +554,15 @@ void Instruction::dropUBImplyingAttrsAndUnknownMetadata(
   CB->removeRetAttrs(UBImplyingAttributes);
 }
 
-void Instruction::dropUBImplyingAttrsAndMetadata() {
+void Instruction::dropUBImplyingAttrsAndMetadata(ArrayRef<unsigned> Keep) {
   // !annotation metadata does not impact semantics.
   // !range, !nonnull and !align produce poison, so they are safe to speculate.
   // !noundef and various AA metadata must be dropped, as it generally produces
   // immediate undefined behavior.
-  unsigned KnownIDs[] = {LLVMContext::MD_annotation, LLVMContext::MD_range,
-                         LLVMContext::MD_nonnull, LLVMContext::MD_align};
+  SmallVector<unsigned> KnownIDs(Keep.size() + 4);
+  KnownIDs.append({LLVMContext::MD_annotation, LLVMContext::MD_range,
+                   LLVMContext::MD_nonnull, LLVMContext::MD_align});
+  append_range(KnownIDs, Keep);
   dropUBImplyingAttrsAndUnknownMetadata(KnownIDs);
 }
 
diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index 34adc7745f67c..7a203638a1d78 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -1698,8 +1698,12 @@ static void hoist(Instruction &I, const DominatorTree *DT, const Loop *CurLoop,
       // The check on hasMetadataOtherThanDebugLoc is to prevent us from burning
       // time in isGuaranteedToExecute if we don't actually have anything to
       // drop.  It is a compile time optimization, not required for correctness.
-      !SafetyInfo->isGuaranteedToExecute(I, DT, CurLoop))
-    I.dropUBImplyingAttrsAndMetadata();
+      !SafetyInfo->isGuaranteedToExecute(I, DT, CurLoop)) {
+    if (ProfcheckDisableMetadataFixes)
+      I.dropUBImplyingAttrsAndMetadata();
+    else
+      I.dropUBImplyingAttrsAndMetadata({LLVMContext::MD_prof});
+  }
 
   if (isa<PHINode>(I))
     // Move the new node to the end of the phi list in the destination block.
diff --git a/llvm/test/Transforms/LICM/hoist-profdata.ll b/llvm/test/Transforms/LICM/hoist-profdata.ll
new file mode 100644
index 0000000000000..18fa1b9f92e8a
--- /dev/null
+++ b/llvm/test/Transforms/LICM/hoist-profdata.ll
@@ -0,0 +1,45 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals --version 2
+; Test that hoisting conditional branches copies the debug and profiling info
+; metadata from the branch being hoisted.
+; RUN: opt -S -passes=licm %s -o - | FileCheck %s
+
+declare i32 @foo()
+
+; to_hoist should get hoisted, and that should not result
+; in a loss of profiling info
+define i32 @hoist_select(i1 %cond, i32 %a, i32 %b) nounwind {
+; CHECK-LABEL: define i32 @hoist_select
+; CHECK-SAME: (i1 [[COND:%.*]], i32 [[A:%.*]], i32 [[B:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TO_HOIST:%.*]] = select i1 [[COND]], i32 [[A]], i32 [[B]], !prof [[PROF0:![0-9]+]]
+; CHECK-NEXT:    br label [[L0:%.*]]
+; CHECK:       L0:
+; CHECK-NEXT:    [[G:%.*]] = call i32 @foo()
+; CHECK-NEXT:    [[SUM:%.*]] = add i32 [[G]], [[TO_HOIST]]
+; CHECK-NEXT:    [[C:%.*]] = icmp eq i32 [[SUM]], 0
+; CHECK-NEXT:    br i1 [[C]], label [[L0]], label [[EXIT:%.*]], !prof [[PROF1:![0-9]+]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[SUM_LCSSA:%.*]] = phi i32 [ [[SUM]], [[L0]] ]
+; CHECK-NEXT:    ret i32 [[SUM_LCSSA]]
+;
+entry:
+  br label %L0
+L0:
+  %g = call i32 @foo()
+  %to_hoist = select i1 %cond, i32 %a, i32 %b, !prof !0
+  %sum = add i32 %g, %to_hoist
+  %c = icmp eq i32 %sum, 0
+  br i1 %c, label %L0, label %exit, !prof !1
+
+exit:
+  ret i32 %sum
+}
+
+!0 = !{!"branch_weights", i32 2, i32 5}
+!1 = !{!"branch_weights", i32 101, i32 189}
+;.
+; CHECK: attributes #[[ATTR0]] = { nounwind }
+;.
+; CHECK: [[PROF0]] = !{!"branch_weights", i32 2, i32 5}
+; CHECK: [[PROF1]] = !{!"branch_weights", i32 101, i32 189}
+;.

mtrofin mentioned this pull request Aug 7, 2025

[licm] clone MD_prof when hoisting conditional branch #152232

Merged

mtrofin force-pushed the users/mtrofin/08-06-_ir_md_prof_is_not_ub-implying branch from 31a3f5c to f0cf2e9 Compare August 7, 2025 21:45

mtrofin force-pushed the users/mtrofin/08-05-_licm_clone_metadata_when_hoisting_conditional_branch branch from 82b2df1 to 9bea77f Compare August 8, 2025 18:22

mtrofin force-pushed the users/mtrofin/08-06-_ir_md_prof_is_not_ub-implying branch 2 times, most recently from 7186d3e to 15e7ab7 Compare August 9, 2025 05:25

mtrofin force-pushed the users/mtrofin/08-05-_licm_clone_metadata_when_hoisting_conditional_branch branch from 9bea77f to ac134fb Compare August 9, 2025 05:25

nikic reviewed Aug 9, 2025

View reviewed changes

llvm/lib/IR/Metadata.cpp Outdated Show resolved Hide resolved

nikic reviewed Aug 9, 2025

View reviewed changes

llvm/test/Transforms/LICM/hoist-phi-metadata.ll Outdated Show resolved Hide resolved

mtrofin force-pushed the users/mtrofin/08-06-_ir_md_prof_is_not_ub-implying branch from 15e7ab7 to df2474e Compare August 9, 2025 23:09

mtrofin force-pushed the users/mtrofin/08-05-_licm_clone_metadata_when_hoisting_conditional_branch branch from ac134fb to cac2eba Compare August 9, 2025 23:09

mtrofin force-pushed the users/mtrofin/08-06-_ir_md_prof_is_not_ub-implying branch from df2474e to d35d9df Compare August 11, 2025 14:47

mtrofin force-pushed the users/mtrofin/08-05-_licm_clone_metadata_when_hoisting_conditional_branch branch from cac2eba to b8aa221 Compare August 11, 2025 14:47

mtrofin force-pushed the users/mtrofin/08-06-_ir_md_prof_is_not_ub-implying branch from d35d9df to 336185b Compare August 11, 2025 21:03

mtrofin marked this pull request as ready for review August 11, 2025 21:34

llvmbot added llvm:ir llvm:transforms labels Aug 11, 2025

mtrofin force-pushed the users/mtrofin/08-06-_ir_md_prof_is_not_ub-implying branch 2 times, most recently from 336ba46 to 6b1a568 Compare August 11, 2025 22:27

mtrofin force-pushed the users/mtrofin/08-05-_licm_clone_metadata_when_hoisting_conditional_branch branch from b8aa221 to 223404b Compare August 11, 2025 22:27

mtrofin requested a review from nikic August 12, 2025 14:10

mtrofin changed the title ~~[ir] MD_prof is not UB-implying~~ [licm] don't drop MD_prof when dropping Aug 12, 2025

mtrofin force-pushed the users/mtrofin/08-05-_licm_clone_metadata_when_hoisting_conditional_branch branch from 223404b to 871de1e Compare August 12, 2025 16:59

mtrofin force-pushed the users/mtrofin/08-06-_ir_md_prof_is_not_ub-implying branch from 6b1a568 to 9487727 Compare August 12, 2025 16:59

This was referenced Aug 12, 2025

[nfc][pgo] Expose traversal of VP metadata elements #153237

Closed

[nfc][pgo] const-ify some APIs in InstrProfSymtab #153284

Open

mtrofin force-pushed the users/mtrofin/08-06-_ir_md_prof_is_not_ub-implying branch from 9487727 to 76de28d Compare August 12, 2025 21:40

mtrofin force-pushed the users/mtrofin/08-05-_licm_clone_metadata_when_hoisting_conditional_branch branch from 871de1e to 8ed35ac Compare August 12, 2025 21:40

mtrofin mentioned this pull request Aug 12, 2025

[JTS] Propagate profile info #153305

Draft

Base automatically changed from users/mtrofin/08-05-_licm_clone_metadata_when_hoisting_conditional_branch to main August 13, 2025 00:01

mtrofin force-pushed the users/mtrofin/08-06-_ir_md_prof_is_not_ub-implying branch from 76de28d to 714993c Compare August 13, 2025 00:02

[ir] MD_prof is not UB-implying

3144e61

mtrofin force-pushed the users/mtrofin/08-06-_ir_md_prof_is_not_ub-implying branch from 714993c to 3144e61 Compare August 13, 2025 05:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[licm] don't drop `MD_prof` when dropping #152420

[licm] don't drop `MD_prof` when dropping #152420

Uh oh!

mtrofin commented Aug 7, 2025

Uh oh!

mtrofin commented Aug 7, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Aug 11, 2025

Uh oh!

llvmbot commented Aug 11, 2025

Uh oh!

Uh oh!

[licm] don't drop MD_prof when dropping #152420

Are you sure you want to change the base?

[licm] don't drop MD_prof when dropping #152420

Uh oh!

Conversation

mtrofin commented Aug 7, 2025

Uh oh!

mtrofin commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Aug 11, 2025

Uh oh!

llvmbot commented Aug 11, 2025

Uh oh!

Uh oh!

[licm] don't drop `MD_prof` when dropping #152420

[licm] don't drop `MD_prof` when dropping #152420

mtrofin commented Aug 7, 2025 •

edited

Loading