-
Notifications
You must be signed in to change notification settings - Fork 122
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
Balar mmio with Vanadis #2428
base: devel
Are you sure you want to change the base?
Balar mmio with Vanadis #2428
Conversation
* Add a new CUDA API id "GPU_PARAM_CONFIG" to support querying kernel function argument size and alignment information from GPGPU-Sim. * Add param "cuda_executable" to BalarMMIO so that it can know the CUDA binary path when running LLVM CUDA code (Vanadis cannot know the host file structure). * Add all the CUDA API implementations needed to link the test program inside tests/vanadisLLVMRISCV. * Minor formatting changes.
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gvoskuilen @feldergast Do we want to keep the prerequisites in this readme or remove them in favor of the list that we test against? Already discussed what testing we want in the nightlies versus weeklies.
src/sst/elements/balar/README.md
Outdated
|
||
- Tested on commit `0f358dda178f96db3b0da88b2b965492c4be187d` | ||
- Use `./configure --prefix=$SST_CORE_HOME --disable-mpi --disable-mem-pools` for sst-core config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@William-An Did you test at all with mem pools enabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have just tried with both mpi and mem pools enabled, and balar can work with these two options.
balar->cuda_ret.is_cuda_call_done = false; | ||
|
||
// Create a DMA request to read the cuda call packet from cache to balar | ||
DMAEngine::DMAEngineControlRegisters dma_registers; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@William-An Did we discuss putting this in memH or vanadis?
@gvoskuilen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have this settled down.
gridDim, | ||
blockDim, | ||
packet->configure_call.sharedMem, | ||
packet->configure_call.stream | ||
(cudaStream_t) packet->configure_call.stream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do CUDA streams work in this framework?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think rodinia 2.0 use CUDA stream, but I can create a test example for this with https://github.com/NVIDIA/cuda-samples/tree/master.
GPU_MALLOC_HOST_RET, | ||
}; | ||
|
||
// Future: Make this into a class with additional serialization methods? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gvoskuilen @feldergast Is this going to be necessary for checkpointing/debug?
# Constans shared across components | ||
network_bw = "25GB/s" | ||
clock = "2GHz" | ||
balar_mmio_testcpu_addr = 4096 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@William-An How configurable are the mmio addresses?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For testcpu, the mmio addresses can be moved around. For using with Vanadis, the address needed to be 0x80100000
as it is specified in the vanadis's hashmap for file descriptor.
clock = "2GHz" | ||
balar_mmio_testcpu_addr = 4096 | ||
balar_mmio_vanadis_addr = 0x80100000 | ||
balar_mmio_size = 1024 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the mmio sizes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Balar does not need a large MMIO size, as CUDA call packet data are passed via pointers. But when using with vanadis, since it will be mmaped by page granularity, address range from 0x80100000
to 0x80100FFF
will be mapped as /dev/balar
.
uint64_t size; | ||
uint64_t offset; | ||
uint8_t value[200]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@William-An If this is related to the array from above, we should find a way to ensure that this is propagated everywhere that relies on it.
@@ -43,7 +48,8 @@ int main( int argc, char* argv[] ) { | |||
|
|||
// Preparing the data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why only five updates? And why is n = 10k?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I set it to 5 as Vanadis is a bit slow dealing with print syscall, and 10k is a number I picked that is large enough to run.
|
||
/** | ||
* @file cuda_runtime_api.h | ||
* @author Weili An ([email protected]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@William-An You should probably remove your email address from these unless you want users bugging you directly. ^-^
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
bb3a75c
to
ef89fe0
Compare
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
Balar mmio with Vanadis