SPMDization D102307 detects incompatible OpenMP runtime calls to abort converting a target region to SPMD mode. Calls to memory allocation/de-allocation routines kmpc_alloc_shared, kmpc_free_shared are incompatible unless they are removed by AAHeapToStack/AAHeapToShared analysis. This patch extends SPMDization detection to include AAHeapToStack/AAHeapToShared analysis results for enlarging the scope of possible SPMDized regions detected.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
llvm/lib/Transforms/IPO/OpenMPOpt.cpp | ||
---|---|---|
2567 | There should only ever be one free call per variable, it's like a stack. This is here just to make sure we don't accidentally remove something that's behind a PHI or something. | |
2614–2621 | We could remove this if we track the free calls separately, but we'd need to have some mapping from the allocation to its corresponding free. |
Test ;)
llvm/lib/Transforms/IPO/OpenMPOpt.cpp | ||
---|---|---|
2567 | Initially we have one free per alloc and the free uses the alloc, transformations can change that, e.g. introduce phi nodes or sink free into conditionals. We need to be conservative if the structure is not as we expect it. | |
3456–3459 | One is sufficient, if the other becomes invalid we don't have to give up. In general, we should not give up on all of kernel info if SPMD-zation can't happen. Also, this is not even needed here ;) | |
3499–3501 | ||
3529 | indicateOptimisticFixpoint is not correct above. |
llvm/lib/Transforms/IPO/OpenMPOpt.cpp | ||
---|---|---|
3498 | For HeapToShared I used lookupAAFor because it returns null if the instance doesn't exist or is invalid. Not sure if that is an issue here. |
Test?
llvm/lib/Transforms/IPO/OpenMPOpt.cpp | ||
---|---|---|
3498 | heap2shared doesn't depend on heap2stack, it could but it doesn't. Here we want getorcreate because there is a real dependence |
Some minor things to address but LGTM.
llvm/lib/Transforms/IPO/AttributorAttributes.cpp | ||
---|---|---|
5718 | Nit: early exit please. | |
llvm/lib/Transforms/IPO/OpenMPOpt.cpp | ||
2554 | Add brief doxygen description please. | |
2567 | Remove the todo, keep the check though. | |
2589–2593 | ||
2600 | same as above. | |
3494 | Nit: Add a message. | |
llvm/test/Transforms/OpenMP/spmdization.ll | ||
68 | Rebase and this is probably going to be a 2 not a 0 anymore. |
Looks like you've got some missing override markers:
llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2588:8: error: 'isAssumedHeapToShared' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override] bool isAssumedHeapToShared(CallBase &CB) const { ^ llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2520:16: note: overridden virtual function is here virtual bool isAssumedHeapToShared(CallBase &CB) const = 0; ^ llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2592:8: error: 'isAssumedHeapToSharedRemovedFree' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override] bool isAssumedHeapToSharedRemovedFree(CallBase &CB) const { ^ llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2524:16: note: overridden virtual function is here virtual bool isAssumedHeapToSharedRemovedFree(CallBase &CB) const = 0; ^ 2 errors generated.
Nit: early exit please.