Repository navigation
Conversation
saleelk
requested review from
a team,
atgutier,
cfreeamd,
dayatsin-amd,
kentrussell and
shwetagkhatri
as code owners
October 7, 2026 05:46
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
saleelk
changed the title
saleelk
changed the base branch from
develop
to
users/saleelk/amdSignalV2-clr-steps
October 7, 2026 05:52
gandryey
approved these changes
Oct 8, 2026
Part 2 of a series moving CLR completion signals to the ROCr batch API and device memory. This part prepares HwQueueTracker for a second signal reuse strategy, which the next part adds for queue completion signals in device memory. No behaviour change. HwQueueTracker becomes an abstract base that keeps the flow shared by any strategy: Create, ActiveSignal, WaitingSignal, CpuWaitForSignal, WaitCurrent, the interrupt/non-interrupt pools and the external and dynamic-queue waits. ActiveSignal and WaitingSignal call hooks for the parts that decide how a signal is proven free and reset: - AcquireSlot: move to the next slot and make its signal reusable. - BeginUse: start the use at init_val. - SetAsyncHandler: register the completion handler for a callback. - IsBusy: whether a signal to wait on may still be in use. - ResetCurrentSignal: roll back a slot after a ROCr failure. LegacySignalTracker implements them with the existing code, moved unchanged: peek two slots ahead and grow, WaitCurrent and WaitNext, replace a signal shared with an event, then signal_silent_store at hand-out. VirtualGPU owns the tracker through std::unique_ptr, so the tracker no longer derives from amd::EmbeddedObject, which forbids heap allocation. Barriers() still returns a reference, so call sites are unchanged. Test (MI300X): - The moved bodies (AcquireSlot, WaitNext, ResetCurrentSignal) are textually identical to the code they replace. - HIP stream/graph test passes; graph_bench --verify passes. - graph_bench --sync times match the previous commit within noise. - hip-tests StreamTest, GraphsTest1, GraphsTest2: failure sets identical to the previous commit with DEBUG_CLR_GRAPH_DEV_SIGNALS=0 and 1 (all pre-existing).
saleelk
force-pushed
the
users/saleelk/amdSignalV2-clr-step2
branch
from
October 10, 2026 01:08
6f8d433 to
ae25a80
Compare
saleelk
force-pushed
the
users/saleelk/amdSignalV2-clr-steps
branch
from
October 10, 2026 01:08
3a2181d to
cef557b
Compare
saleelk
merged commit Oct 10, 2026
6ca67d7
into
users/saleelk/amdSignalV2-clr-steps
77 of 91 checks passed
saleelk
added a commit
that referenced
this pull request
Oct 12, 2026
…12991) Stacked on #12990 (part 1), which is stacked on #12988 (ROCr), so this PR shows only the part 2 commit. It will be retargeted once those merge. Part 2 of a series moving CLR completion signals to the ROCr batch API and device memory. This part prepares HwQueueTracker for a second signal reuse strategy, which the next part adds for queue completion signals in device memory. No behaviour change. HwQueueTracker becomes an abstract base that keeps the flow shared by any strategy: Create, ActiveSignal, WaitingSignal, CpuWaitForSignal, WaitCurrent, the interrupt/non-interrupt pools and the external and dynamic-queue waits. ActiveSignal and WaitingSignal call hooks for the parts that decide how a signal is proven free and reset: - AcquireSlot: move to the next slot and make its signal reusable. - BeginUse: start the use at init_val. - SetAsyncHandler: register the completion handler for a callback. - IsBusy: whether a signal to wait on may still be in use. - ResetCurrentSignal: roll back a slot after a ROCr failure. LegacySignalTracker implements them with the existing code, moved unchanged: peek two slots ahead and grow, WaitCurrent and WaitNext, replace a signal shared with an event, then signal_silent_store at hand-out. VirtualGPU owns the tracker through std::unique_ptr, so the tracker no longer derives from amd::EmbeddedObject, which forbids heap allocation. Barriers() still returns a reference, so call sites are unchanged. Test (MI300X): - The moved bodies (AcquireSlot, WaitNext, ResetCurrentSignal) are textually identical to the code they replace. - HIP stream/graph test passes; graph_bench --verify passes. - graph_bench --sync times match the previous commit within noise. - hip-tests StreamTest, GraphsTest1, GraphsTest2: failure sets identical to the previous commit with DEBUG_CLR_GRAPH_DEV_SIGNALS=0 and 1 (all pre-existing).
saleelk
added a commit
that referenced
this pull request
Oct 13, 2026
…QueueTracker (#12990) Stacked on #12988 (ROCr batch signal API). It will be retargeted to `develop` once #12988 merges. This includes the HwQueueTracker split reviewed in #12991. Part 1 of a series moving CLR completion signals to the ROCr batch API and device memory. This part places the hip graph pool signals in device memory, and prepares HwQueueTracker for a second signal reuse strategy, which a later part adds for queue completion signals in device memory. Graph pool signals: Use the ROCr batch signal API for the signals CLR pre-creates for the segmented hip graph path, so each pool set comes from one allocation, and place those signals in device memory where the host can reach it. - rocrctx: load hsa_amd_signal_batch_create/destroy and add Hsa::signal_batch_create/destroy wrappers. GET_ROCR_OPTIONAL_SYMBOL now casts through decltype like GET_ROCR_SYMBOL, so hsa_amd_queue_create no longer needs a hand-written function typedef, and GET_ROCR_OPTIONAL_SYMBOL_DECLTYPE, which did the same, goes away. - Device::CreateSignals(count, initial_value, attributes, batched, device_memory, signals, in_device_memory): when batched, create v1 signals as one batch and release the batch handle immediately; device_memory takes the batch from the device's uncached fine-grained pool (gpu_ext_fine_grained_segment_), which is only reachable through a batch. ProfilingSignal still destroys each signal individually, which ROCr allows in any order. Falls back to per-signal host signals if the batch fails, and reports where the signals were placed. - amd::Device::CreateHwEvents gains a batched argument. Graph pool sets are batched: GraphSignalManager::Prepopulate and the AcquireSet fallback, whose set joins the pool when released. The non-pooled launch path keeps per-signal creation unless the signals go to device memory. - DEBUG_CLR_GRAPH_DEV_SIGNALS (default on) places graph signals in device memory when the device has a large BAR, so the barrier packets between graph segments poll device-local memory instead of host memory. Each ProfilingSignal records where it lives. ResetHwEvents re-arms device signals with direct stores and one HDP flush (Device::FlushSignalStores), issued only when ROCr's HSA_AMD_AGENT_INFO_HOST_STORES_NEED_HDP_FLUSH reports that host stores need it; host signals keep signal_silent_store. - The flush is skipped when the device's AQL rings are in device memory (Device::AqlRingsInDeviceMemory, now also used by acquireQueue). Only packets on this device's rings wait on graph signals, and they reach a device-memory ring through the same BAR after the stores, in order, as dispatch already relies on for kernel arguments. Queue completion signals are unchanged. All batches are v1 (128-byte slots); no signal hints are emitted. A host batch costs about 60 us regardless of count on MI300X and a device batch 86-158 us, versus under 1 us per pooled signal, so only pooled sets are batched; the AcquireSet fallback pays it once per pool growth. HwQueueTracker split (no behaviour change): HwQueueTracker becomes an abstract base that keeps the flow shared by any strategy: Create, ActiveSignal, WaitingSignal, CpuWaitForSignal, WaitCurrent, the interrupt/non-interrupt pools and the external and dynamic-queue waits. ActiveSignal and WaitingSignal call hooks for the parts that decide how a signal is proven free and reset: - AcquireSlot: move to the next slot and make its signal reusable. - BeginUse: start the use at init_val. - SetAsyncHandler: register the completion handler for a callback. - IsBusy: whether a signal to wait on may still be in use. - ResetCurrentSignal: roll back a slot after a ROCr failure. LegacySignalTracker implements them with the existing code, moved unchanged: peek two slots ahead and grow, WaitCurrent and WaitNext, replace a signal shared with an event, then signal_silent_store at hand-out. VirtualGPU owns the tracker through std::unique_ptr, so the tracker no longer derives from amd::EmbeddedObject, which forbids heap allocation. Barriers() still returns a reference, so call sites are unchanged. Performance (MI300X, end to end, median of 3 alternating rounds, host vs device graph signals, AQL rings in device memory): - graph_bench --sync: paths4 98 -> 64 us (64 nodes), 318 -> 214 us (256), 1263 -> 815 us (1024); paths2 -19% to -21%; full4 up to -13%; full2 and straight unchanged. - MoE dual-stream graph: 5 layers 344 -> 299 us (original order), 308 -> 284 us (reversed); 93 layers 6289 -> 5347 us and 5531 -> 5079 us; single-stream unchanged. Test (MI300X): - graph_bench --verify and hipGraphCollapseCorrectness (collapsed and multi-stream) pass with DEBUG_CLR_GRAPH_DEV_SIGNALS=0 and 1, with AQL rings in device and in system memory. - HIP stream/graph test with an LD_PRELOAD counter, under DEBUG_HIP_GRAPH_MIN_OVERLAP=0 and DEBUG_HIP_GRAPH_SEGMENT_SCHEDULING=1 and 2: graph pool sets, including AcquireSet fallbacks, come from batches; all results correct with the flag off and on. - The moved tracker bodies (AcquireSlot, WaitNext, ResetCurrentSignal) are textually identical to the code they replace. - hip-tests StreamTest, GraphsTest1, GraphsTest2: failure sets identical with the flag off and on (all pre-existing). - rocrctx.cpp, rocdevice.cpp and rocvirtual.cpp compile with ROCR_DYN_DLL defined.
saleelk
added a commit
that referenced
this pull request
Oct 21, 2026
…QueueTracker (#13424) Builds on #12988 (ROCr batch signal API), now merged. This replaces #12990, which was merged into `amdSignalV2` and split back out so #12988 stayed ROCr-only. It includes the HwQueueTracker split reviewed in #12991. Part 1 of a series moving CLR completion signals to the ROCr batch API and device memory. This part places the hip graph pool signals in device memory, and prepares HwQueueTracker for a second signal reuse strategy, which a later part adds for queue completion signals in device memory. Graph pool signals: Use the ROCr batch signal API for the signals CLR pre-creates for the segmented hip graph path, so each pool set comes from one allocation, and place those signals in device memory where the host can reach it. - rocrctx: load hsa_amd_signal_batch_create/destroy and add Hsa::signal_batch_create/destroy wrappers. GET_ROCR_OPTIONAL_SYMBOL now casts through decltype like GET_ROCR_SYMBOL, so hsa_amd_queue_create no longer needs a hand-written function typedef, and GET_ROCR_OPTIONAL_SYMBOL_DECLTYPE, which did the same, goes away. When ROCr is loaded dynamically (ROCR_DYN_DLL) the batch entry points are optional: against a ROCr older than interface 1.36, Hsa::signal_batch_create fails and CreateSignals falls back to per-signal creation. A build linked against ROCr requires them, as it already requires hsa_amd_queue_create. - Device::CreateSignals(count, initial_value, attributes, batched, device_memory, signals, in_device_memory): when batched, create v1 signals as one batch and release the batch handle immediately; device_memory takes the batch from Device::device_signal_pool_, which is only reachable through a batch. That is the first device pool ROCr reports with HSA_AMD_MEMORY_POOL_INFO_SIGNAL_CAPABLE: the uncached pool on gfx94x/gfx95x, the coarse-grained pool on gfx125x. Without one, or with a ROCr that lacks the query, signals stay in host memory. ProfilingSignal still destroys each signal individually, which ROCr allows in any order. Falls back to per-signal host signals if the batch fails, and reports where the signals were placed. - amd::Device::CreateHwEvents gains a batched argument. Graph pool sets are batched: GraphSignalManager::Prepopulate and the AcquireSet fallback, whose set joins the pool when released. The non-pooled launch path (legacy recursive child graphs) creates its set on every launch, so it keeps per-signal host signals rather than pay for a device batch each time. - DEBUG_CLR_GRAPH_DEV_SIGNALS (default on) places pooled graph signals in device memory when the device has a large BAR, so the barrier packets between graph segments poll device-local memory instead of host memory. Each ProfilingSignal records where it lives. ResetHwEvents re-arms device signals with direct stores and one HDP flush (Device::FlushSignalStores), issued only when ROCr's HSA_AMD_AGENT_INFO_PCIE_STORES_NEED_FLUSH reports that the device's memory is reached over PCIe; host signals keep signal_silent_store. - Graph signals stay in host memory, and AQL rings in system memory (AqlRingsInDeviceMemory), while a profiler tool is attached, i.e. when rocprofiler-register accepts HIP's API tables (amd::activity_prof::tool_attached). The tool scans and rewrites packets from the host, and rocprofiler-sdk's queue interposition adds to and subtracts from an application's completion signals with host atomics, which are not atomic on device memory over PCIe, and its completion monitor can still poll a signal after the graph destroys it, when the batch's device-memory arena is unmapped. hip-graph-forkjoin-deadlock segfaulted that way. - The flush is skipped when the device's AQL rings are in device memory (Device::AqlRingsInDeviceMemory, now also used by acquireQueue). Only packets on this device's rings wait on graph signals, and they reach a device-memory ring through the same BAR after the stores, in order, as dispatch already relies on for kernel arguments. The predicate is the ring placement every such queue actually has: acquireQueue requests it, and ROCr fails the request rather than fall back to a system-memory ring. Queue completion signals are unchanged. All batches are v1 (128-byte slots); no signal hints are emitted. A host batch costs about 60 us regardless of count on MI300X and a device batch 86-158 us, versus under 1 us per pooled signal, so only pooled sets are batched; the AcquireSet fallback pays it once per pool growth. HwQueueTracker split (no behaviour change): HwQueueTracker becomes an abstract base that keeps the flow shared by any strategy: Create, ActiveSignal, WaitingSignal, CpuWaitForSignal, WaitCurrent, the interrupt/non-interrupt pools and the external and dynamic-queue waits. ActiveSignal and WaitingSignal call hooks for the parts that decide how a signal is proven free and reset: - AcquireSlot: move to the next slot and make its signal reusable. - BeginUse: start the use at init_val. - SetAsyncHandler: register the completion handler for a callback. - IsBusy: whether a signal to wait on may still be in use. - ResetCurrentSignal: roll back a slot after a ROCr failure. LegacySignalTracker implements them with the existing code, moved unchanged: peek two slots ahead and grow, WaitCurrent and WaitNext, replace a signal shared with an event, then signal_silent_store at hand-out. VirtualGPU owns the tracker through std::unique_ptr, so the tracker no longer derives from amd::EmbeddedObject, which forbids heap allocation. Barriers() still returns a reference, so call sites are unchanged. Performance (MI300X, end to end, median of 3 alternating rounds, host vs device graph signals, AQL rings in device memory): - graph_bench --sync: paths4 98 -> 64 us (64 nodes), 318 -> 214 us (256), 1263 -> 815 us (1024); paths2 -19% to -21%; full4 up to -13%; full2 and straight unchanged. - MoE dual-stream graph: 5 layers 344 -> 299 us (original order), 308 -> 284 us (reversed); 93 layers 6289 -> 5347 us and 5531 -> 5079 us; single-stream unchanged. Test (MI300X): - graph_bench --verify and hipGraphCollapseCorrectness (collapsed and multi-stream) pass with DEBUG_CLR_GRAPH_DEV_SIGNALS=0 and 1, with AQL rings in device and in system memory. - HIP stream/graph test with an LD_PRELOAD counter, under DEBUG_HIP_GRAPH_MIN_OVERLAP=0 and DEBUG_HIP_GRAPH_SEGMENT_SCHEDULING=1 and 2: graph pool sets, including AcquireSet fallbacks, come from batches; all results correct with the flag off and on. - The moved tracker bodies (AcquireSlot, WaitNext, ResetCurrentSignal) are textually identical to the code they replace. - hip-tests StreamTest, GraphsTest1, GraphsTest2: failure sets identical with the flag off and on (all pre-existing). - rocrctx.cpp, rocdevice.cpp and rocvirtual.cpp compile with ROCR_DYN_DLL defined.
dgaliffiAMD
pushed a commit
that referenced
this pull request
Oct 21, 2026
…QueueTracker (#13424) Builds on #12988 (ROCr batch signal API), now merged. This replaces #12990, which was merged into `amdSignalV2` and split back out so #12988 stayed ROCr-only. It includes the HwQueueTracker split reviewed in #12991. Part 1 of a series moving CLR completion signals to the ROCr batch API and device memory. This part places the hip graph pool signals in device memory, and prepares HwQueueTracker for a second signal reuse strategy, which a later part adds for queue completion signals in device memory. Graph pool signals: Use the ROCr batch signal API for the signals CLR pre-creates for the segmented hip graph path, so each pool set comes from one allocation, and place those signals in device memory where the host can reach it. - rocrctx: load hsa_amd_signal_batch_create/destroy and add Hsa::signal_batch_create/destroy wrappers. GET_ROCR_OPTIONAL_SYMBOL now casts through decltype like GET_ROCR_SYMBOL, so hsa_amd_queue_create no longer needs a hand-written function typedef, and GET_ROCR_OPTIONAL_SYMBOL_DECLTYPE, which did the same, goes away. When ROCr is loaded dynamically (ROCR_DYN_DLL) the batch entry points are optional: against a ROCr older than interface 1.36, Hsa::signal_batch_create fails and CreateSignals falls back to per-signal creation. A build linked against ROCr requires them, as it already requires hsa_amd_queue_create. - Device::CreateSignals(count, initial_value, attributes, batched, device_memory, signals, in_device_memory): when batched, create v1 signals as one batch and release the batch handle immediately; device_memory takes the batch from Device::device_signal_pool_, which is only reachable through a batch. That is the first device pool ROCr reports with HSA_AMD_MEMORY_POOL_INFO_SIGNAL_CAPABLE: the uncached pool on gfx94x/gfx95x, the coarse-grained pool on gfx125x. Without one, or with a ROCr that lacks the query, signals stay in host memory. ProfilingSignal still destroys each signal individually, which ROCr allows in any order. Falls back to per-signal host signals if the batch fails, and reports where the signals were placed. - amd::Device::CreateHwEvents gains a batched argument. Graph pool sets are batched: GraphSignalManager::Prepopulate and the AcquireSet fallback, whose set joins the pool when released. The non-pooled launch path (legacy recursive child graphs) creates its set on every launch, so it keeps per-signal host signals rather than pay for a device batch each time. - DEBUG_CLR_GRAPH_DEV_SIGNALS (default on) places pooled graph signals in device memory when the device has a large BAR, so the barrier packets between graph segments poll device-local memory instead of host memory. Each ProfilingSignal records where it lives. ResetHwEvents re-arms device signals with direct stores and one HDP flush (Device::FlushSignalStores), issued only when ROCr's HSA_AMD_AGENT_INFO_PCIE_STORES_NEED_FLUSH reports that the device's memory is reached over PCIe; host signals keep signal_silent_store. - Graph signals stay in host memory, and AQL rings in system memory (AqlRingsInDeviceMemory), while a profiler tool is attached, i.e. when rocprofiler-register accepts HIP's API tables (amd::activity_prof::tool_attached). The tool scans and rewrites packets from the host, and rocprofiler-sdk's queue interposition adds to and subtracts from an application's completion signals with host atomics, which are not atomic on device memory over PCIe, and its completion monitor can still poll a signal after the graph destroys it, when the batch's device-memory arena is unmapped. hip-graph-forkjoin-deadlock segfaulted that way. - The flush is skipped when the device's AQL rings are in device memory (Device::AqlRingsInDeviceMemory, now also used by acquireQueue). Only packets on this device's rings wait on graph signals, and they reach a device-memory ring through the same BAR after the stores, in order, as dispatch already relies on for kernel arguments. The predicate is the ring placement every such queue actually has: acquireQueue requests it, and ROCr fails the request rather than fall back to a system-memory ring. Queue completion signals are unchanged. All batches are v1 (128-byte slots); no signal hints are emitted. A host batch costs about 60 us regardless of count on MI300X and a device batch 86-158 us, versus under 1 us per pooled signal, so only pooled sets are batched; the AcquireSet fallback pays it once per pool growth. HwQueueTracker split (no behaviour change): HwQueueTracker becomes an abstract base that keeps the flow shared by any strategy: Create, ActiveSignal, WaitingSignal, CpuWaitForSignal, WaitCurrent, the interrupt/non-interrupt pools and the external and dynamic-queue waits. ActiveSignal and WaitingSignal call hooks for the parts that decide how a signal is proven free and reset: - AcquireSlot: move to the next slot and make its signal reusable. - BeginUse: start the use at init_val. - SetAsyncHandler: register the completion handler for a callback. - IsBusy: whether a signal to wait on may still be in use. - ResetCurrentSignal: roll back a slot after a ROCr failure. LegacySignalTracker implements them with the existing code, moved unchanged: peek two slots ahead and grow, WaitCurrent and WaitNext, replace a signal shared with an event, then signal_silent_store at hand-out. VirtualGPU owns the tracker through std::unique_ptr, so the tracker no longer derives from amd::EmbeddedObject, which forbids heap allocation. Barriers() still returns a reference, so call sites are unchanged. Performance (MI300X, end to end, median of 3 alternating rounds, host vs device graph signals, AQL rings in device memory): - graph_bench --sync: paths4 98 -> 64 us (64 nodes), 318 -> 214 us (256), 1263 -> 815 us (1024); paths2 -19% to -21%; full4 up to -13%; full2 and straight unchanged. - MoE dual-stream graph: 5 layers 344 -> 299 us (original order), 308 -> 284 us (reversed); 93 layers 6289 -> 5347 us and 5531 -> 5079 us; single-stream unchanged. Test (MI300X): - graph_bench --verify and hipGraphCollapseCorrectness (collapsed and multi-stream) pass with DEBUG_CLR_GRAPH_DEV_SIGNALS=0 and 1, with AQL rings in device and in system memory. - HIP stream/graph test with an LD_PRELOAD counter, under DEBUG_HIP_GRAPH_MIN_OVERLAP=0 and DEBUG_HIP_GRAPH_SEGMENT_SCHEDULING=1 and 2: graph pool sets, including AcquireSet fallbacks, come from batches; all results correct with the flag off and on. - The moved tracker bodies (AcquireSlot, WaitNext, ResetCurrentSignal) are textually identical to the code they replace. - hip-tests StreamTest, GraphsTest1, GraphsTest2: failure sets identical with the flag off and on (all pre-existing). - rocrctx.cpp, rocdevice.cpp and rocvirtual.cpp compile with ROCR_DYN_DLL defined.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #12990 (part 1), which is stacked on #12988 (ROCr), so this PR shows only the part 2 commit. It will be retargeted once those merge.
Part 2 of a series moving CLR completion signals to the ROCr batch API
and device memory. This part prepares HwQueueTracker for a second
signal reuse strategy, which the next part adds for queue completion
signals in device memory. No behaviour change.
HwQueueTracker becomes an abstract base that keeps the flow shared by
any strategy: Create, ActiveSignal, WaitingSignal, CpuWaitForSignal,
WaitCurrent, the interrupt/non-interrupt pools and the external and
dynamic-queue waits. ActiveSignal and WaitingSignal call hooks for the
parts that decide how a signal is proven free and reset:
LegacySignalTracker implements them with the existing code, moved
unchanged: peek two slots ahead and grow, WaitCurrent and WaitNext,
replace a signal shared with an event, then signal_silent_store at
hand-out.
VirtualGPU owns the tracker through std::unique_ptr, so the tracker no
longer derives from amd::EmbeddedObject, which forbids heap allocation.
Barriers() still returns a reference, so call sites are unchanged.
Test (MI300X):
textually identical to the code they replace.
to the previous commit with DEBUG_CLR_GRAPH_DEV_SIGNALS=0 and 1 (all
pre-existing).