Skip to content

Conversation

teresajohnson
Copy link
Contributor

Address post-commit review feedback for PR139092 (and fix another
instance of the same code). Save and restore option values via a saved
bool value, instead of invoking cl::ResetAllOptionOccurrences.

Address post-commit review feedback for PR139092 (and fix another
instance of the same code). Save and restore option values via a saved
bool value, instead of invoking cl::ResetAllOptionOccurrences.
@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label May 8, 2025
@teresajohnson teresajohnson requested a review from snehasish May 8, 2025 17:53
@llvmbot
Copy link
Member

llvmbot commented May 8, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Teresa Johnson (teresajohnson)

Changes

Address post-commit review feedback for PR139092 (and fix another
instance of the same code). Save and restore option values via a saved
bool value, instead of invoking cl::ResetAllOptionOccurrences.


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

1 Files Affected:

  • (modified) llvm/unittests/Analysis/MemoryProfileInfoTest.cpp (+8-4)
diff --git a/llvm/unittests/Analysis/MemoryProfileInfoTest.cpp b/llvm/unittests/Analysis/MemoryProfileInfoTest.cpp
index 2943631150650..170dca0ebcc53 100644
--- a/llvm/unittests/Analysis/MemoryProfileInfoTest.cpp
+++ b/llvm/unittests/Analysis/MemoryProfileInfoTest.cpp
@@ -88,14 +88,17 @@ TEST_F(MemoryProfileInfoTest, GetAllocType) {
                  100);
 
   // Make sure the option for detecting hot allocations is set.
+  bool OrigMemProfUseHotHints = MemProfUseHotHints;
   MemProfUseHotHints = true;
+
   // Test Hot
   // More accesses per byte per sec than hot threshold is hot.
   EXPECT_EQ(getAllocType(HotTotalLifetimeAccessDensityThreshold + 1, AllocCount,
                          ColdTotalLifetimeThreshold + 1),
             AllocationType::Hot);
-  // Undo the manual set of the option above.
-  cl::ResetAllOptionOccurrences();
+
+  // Restore original option value.
+  MemProfUseHotHints = OrigMemProfUseHotHints;
 
   // Without MemProfUseHotHints (default) we should treat simply as NotCold.
   EXPECT_EQ(getAllocType(HotTotalLifetimeAccessDensityThreshold + 1, AllocCount,
@@ -590,12 +593,13 @@ declare dso_local noalias noundef i8* @malloc(i64 noundef)
   ASSERT_NE(Call, nullptr);
 
   // Specify that all non-cold contexts should be kept.
+  bool OrigMemProfKeepAllNotColdContexts = MemProfKeepAllNotColdContexts;
   MemProfKeepAllNotColdContexts = true;
 
   Trie.buildAndAttachMIBMetadata(Call);
 
-  // Undo the manual set of the MemProfKeepAllNotColdContexts above.
-  cl::ResetAllOptionOccurrences();
+  // Restore original option value.
+  MemProfKeepAllNotColdContexts = OrigMemProfKeepAllNotColdContexts;
 
   EXPECT_FALSE(Call->hasFnAttr("memprof"));
   EXPECT_TRUE(Call->hasMetadata(LLVMContext::MD_memprof));

Copy link

@snehasish snehasish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@teresajohnson teresajohnson merged commit c526683 into llvm:main May 8, 2025
8 of 11 checks passed
lenary added a commit to lenary/llvm-project that referenced this pull request May 9, 2025
* main: (420 commits)
  [AArch64] Merge scaled and unscaled narrow zero stores (llvm#136705)
  [RISCV] One last migration to getInsertSubvector [nfc]
  [flang][OpenMP] Update `do concurrent` mapping pass to use `fir.do_concurrent` op (llvm#138489)
  [MLIR][LLVM] Fix llvm.mlir.global mismatching print and parser order (llvm#138986)
  [lld][NFC] Fix minor typo in docs (llvm#138898)
  [RISCV] Migrate getConstant indexed insert/extract subvector to new API (llvm#139111)
  GlobalISel: Translate minimumnum and maximumnum (llvm#139106)
  [MemProf] Simplify unittest save and restore of options (llvm#139117)
  [BOLT][AArch64] Patch functions targeted by optional relocs (llvm#138750)
  [Coverage] Support -fprofile-list for cold function coverage (llvm#136333)
  Remove unused forward decl (llvm#139108)
  [AMDGPU][NFC] Get rid of OPW constants. (llvm#139074)
  [CIR] Upstream extract op for VectorType (llvm#138413)
  [mlir][xegpu] Handle scalar uniform ops in SIMT distribution.  (llvm#138593)
  [GlobalISel][AMDGPU] Fix handling of v2i128 type for AND, OR, XOR (llvm#138574)
  AMDGPU][True16][CodeGen] FP_Round f64 to f16 in true16 (llvm#128911)
  Reland [Clang] Deprecate `__is_trivially_relocatable` (llvm#139061)
  [HLSL][NFC] Stricter Overload Tests (clamp,max,min,pow) (llvm#138993)
  [MLIR] Fixing the memref linearization size computation for non-packed memref (llvm#138922)
  [TableGen][NFC] Use early exit to simplify large block in emitAction. (llvm#138220)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:analysis Includes value tracking, cost tables and constant folding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants