Test for checking if the mapping is performed correctly. This is a test initially included in Patch https://reviews.llvm.org/D29905
Details
- Reviewers
Hahnfeld carlo.bertolli caomhin ABataev - Commits
- rG965c7e9c6e17: [OpenMP] Add an additional test for D34888
rGb379ba6a62ef: [OpenMP] Add an additional test for D34888
rG9db6e861acca: [OpenMP] Add an additional test for D34888
rC314303: [OpenMP] Add an additional test for D34888
rC314228: [OpenMP] Add an additional test for D34888
rC314210: [OpenMP] Add an additional test for D34888
rL314303: [OpenMP] Add an additional test for D34888
rL314228: [OpenMP] Add an additional test for D34888
rL314210: [OpenMP] Add an additional test for D34888
Diff Detail
- Repository
- rL LLVM
Event Timeline
LGTM in general.
test/OpenMP/target_map_codegen.cpp | ||
---|---|---|
4845 | Please break the line to make it easier to read. |
test/OpenMP/target_map_codegen.cpp | ||
---|---|---|
4845 | Better, but the line still wraps. I'd split it again somewhere between -emit-llvm and -fopenmp. |
Hi Doru,
if I remember correctly I submitted D34888 for a crash when mapping a scalar value with nested regions.
I've marked another test in this file that the codegen for tofrom is correct. So I don't know if this test checks some other conditions?
Jonas
test/OpenMP/target_map_codegen.cpp | ||
---|---|---|
1773 | tofrom is tested here... |
Hi Jonas,
The test is verifying whether the parameter is passed to the kernel correctly. I believe it was not passed as a reference before the patch.
In addition to that, something that was in my previous patch is related to this code:
DSAStack->checkMappableExprComponentListsForDeclAtLevel( D, Level, [&](OMPClauseMappableExprCommon::MappableExprComponentListRef
In particular with the Level variable. Should the Level variable actually be Level + 1 in this case?
Thanks,
--Doru
Ah, right: This isn't checked anywhere before. Maybe add a comment about what's tested here?
Do we want to check the rest of the codegen with a focus that the variable is passed as a reference?
In addition to that, something that was in my previous patch is related to this code:
DSAStack->checkMappableExprComponentListsForDeclAtLevel( D, Level, [&](OMPClauseMappableExprCommon::MappableExprComponentListRefIn particular with the Level variable. Should the Level variable actually be Level + 1 in this case?
I'm not sure, the current public clang-ykt has Level: https://github.com/clang-ykt/clang/blob/d181aed/lib/Sema/SemaOpenMP.cpp#L1361
tofrom is tested here...