Skip to content

[lldb][NFC] Remove unused macro ENABLE_MEMORY_CACHING #142231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 2, 2025

Conversation

bulbazord
Copy link
Member

This macro does not do what is described. The only thing it actually does control is whether or not the process's memory cache gets flushed when writing to an address. It does not override the setting target.process.disable-memory-cache.

Instead of making it work as intended, I chose to remove the macro. I don't see much value in being able to override the setting with a preprocessor macro.

This macro does not do what is described. The only thing it actually
does control is whether or not the process's memory cache gets flushed
when writing to an address. It does not override the setting
`target.process.disable-memory-cache`.

Instead of making it work as intended, I chose to remove the macro. I
don't see much value in being able to override the setting with a
preprocessor macro.
@bulbazord bulbazord requested a review from JDevlieghere as a code owner May 30, 2025 22:51
@llvmbot llvmbot added the lldb label May 30, 2025
@llvmbot
Copy link
Member

llvmbot commented May 30, 2025

@llvm/pr-subscribers-lldb

Author: Alex Langford (bulbazord)

Changes

This macro does not do what is described. The only thing it actually does control is whether or not the process's memory cache gets flushed when writing to an address. It does not override the setting target.process.disable-memory-cache.

Instead of making it work as intended, I chose to remove the macro. I don't see much value in being able to override the setting with a preprocessor macro.


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

1 Files Affected:

  • (modified) lldb/source/Target/Process.cpp (-12)
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index c377feec86c16..84299f5f9a775 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -80,16 +80,6 @@ using namespace lldb;
 using namespace lldb_private;
 using namespace std::chrono;
 
-// Comment out line below to disable memory caching, overriding the process
-// setting target.process.disable-memory-cache
-#define ENABLE_MEMORY_CACHING
-
-#ifdef ENABLE_MEMORY_CACHING
-#define DISABLE_MEM_CACHE_DEFAULT false
-#else
-#define DISABLE_MEM_CACHE_DEFAULT true
-#endif
-
 class ProcessOptionValueProperties
     : public Cloneable<ProcessOptionValueProperties, OptionValueProperties> {
 public:
@@ -2297,9 +2287,7 @@ size_t Process::WriteMemory(addr_t addr, const void *buf, size_t size,
   if (ABISP abi_sp = GetABI())
     addr = abi_sp->FixAnyAddress(addr);
 
-#if defined(ENABLE_MEMORY_CACHING)
   m_memory_cache.Flush(addr, size);
-#endif
 
   if (buf == nullptr || size == 0)
     return 0;

@bulbazord bulbazord merged commit 7365f02 into llvm:main Jun 2, 2025
11 of 12 checks passed
@bulbazord bulbazord deleted the unused-define-process branch June 2, 2025 18:15
sallto pushed a commit to sallto/llvm-project that referenced this pull request Jun 3, 2025
This macro does not do what is described. The only thing it actually
does control is whether or not the process's memory cache gets flushed
when writing to an address. It does not override the setting
`target.process.disable-memory-cache`.

Instead of making it work as intended, I chose to remove the macro. I
don't see much value in being able to override the setting with a
preprocessor macro.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants