[GPU] Ignore deallocations in shared memory reuse pass #20149
Conversation
Deallocations don't affect the liveness range and can be ignored. Additionally adds the pass to LLVMGPU pipelines as this should be free to run everywhere.
qedawkins
requested review from
MaheshRavishankar,
kuhar,
Groverkss and
antiagainst
as code owners
| @@ -45,6 +45,10 @@ static LivenessRange getLivenessRange(memref::AllocOp alloc, | |||
| workList.push_back(user); | |||
| continue; | |||
| } | |||
| // Skip deallocations as they don't affect the liveness range. | |||
| if (isa<memref::DeallocOp>(user)) { | |||
There was a problem hiding this comment.
Could we just use alloca for shared memory and not have deallocs at all?
There was a problem hiding this comment.
We could, but that's a decent amount of churn for not much benefit that I see. The deallocs are dropped at a later point though, I could just move that pattern up earlier.
There was a problem hiding this comment.
dropping a dealloc is basically saying the alloc is really an alloca. I understand the churn is an issue, but this is precisely the difference between alloca and alloc. We are really abusing alloc here.
There was a problem hiding this comment.
Yeah that is true... ok I can look into changing it.
There was a problem hiding this comment.
So most of the LLVMGPU side looks to work fine with alloca, but SPIR-V only supports memref.alloc with workgroup scope. I'll see if I can keep support for both for the time being then.
There was a problem hiding this comment.
This PR makes the switch for the two main pipelines we're developing: #20167
Some of the CUDA stuff is relying on functions that only support alloc upstream that I don't feel like going and changing.
|
It looks like there are regressions from enabling this pass. Will have to investigate more before landing. |
|
So, the thing is "dealloc doesn't affect the liveness range" isn't true. Dealloc terminates a liveness range and causes that alloc()d memory to be reusable |
Deallocations don't affect the liveness range and can be ignored. Additionally adds the pass to LLVMGPU pipelines as this should be free to run everywhere.