Fixes a host function called from host device function. #1338
Conversation
…U cases where it should not. This silences many warnings about calling std::vector::operator[] when not appropriate.
There was a problem hiding this comment.
Thanks @BradWhitlock!
Note: I think this PR is related to #1196
Tag: @rhornung67
| abort(); | ||
| #else | ||
| // Always return a value. | ||
| return buf[pos]; | ||
| #endif | ||
| #endif | ||
| #else | ||
| // Always return a value. | ||
| return buf[pos]; | ||
| #endif |
There was a problem hiding this comment.
Is this change meant to be more explicit?
My read is that the previous behavior always called return buf[pos] (regardless of whether AXOM_DEVICE_CODE is defined) and the updated behavior does the same.
There was a problem hiding this comment.
I suppose I could have removed the comment " Always return a value". The problem for the CUDA/HIP branches of these functions was that an STLVectorIndirection was passed into the IndexedIndirection and the operator[] it ends up using is not a host-device function and never could be, resulting in a lot of compilation warnings on rzansel. This small change skips calling buf[pos] for the CUDA/HIP branches since they abort first anyway.
| #else | ||
| // Always return a value. | ||
| return buf[pos]; | ||
| #endif | ||
| #endif | ||
| #else | ||
| // Always return a value. | ||
| return buf[pos]; | ||
| #endif |
There was a problem hiding this comment.
Same question as for getIndirection() a few lines up
|
@rhornung67 -- would you be ok w/ merging this PR and then renaming it/incorporating it into the work you've done in #1196? |
|
@BradWhitlock a while back, I was toying around with a related issue (see #1196). Would that be an easier way to fix this type of issue? |
Sure. I need to get back to that, but I keep getting distracted to other things. @BradWhitlock I'm going to tag this PR in my PR so I can remember what's what. |
|
@rhornung67 - Adding a full-blown axom::numeric_limits<> sounds useful to me. I wasn't really aware of that work in #1196 so I used a related axom::numerics::floating_point_limits<>::max() function that was in the code today. I changed the numeric_limits in 2 places that were marked AXOM_HOST_DEVICE. |
Minor compilation warning removal
Note - there are still many more warnings where host code is called from host-device functions. This matters for CUDA/HIP systems where functions need to be available on the GPU, though not all Axom functionality needs to probably be on the GPU. It seems like AXOM_HOST_DEVICE support is being added gradually to Axom so we have a mix of these warnings. The worst offenders are slam (various Maps) and spin.
I looked at some of the slam issues that caused the host from host_device warning to appear. Many come from constructors/destructors and other methods for maps and iterators not yet having AXOM_HOST_DEVICE. Should they even? I did not include those changes because the changes started to feel like a slippery slope.