This is an archive of the discontinued LLVM Phabricator instance.

[VectorCombine] Avoid crossing address space boundaries.
ClosedPublic

Authored by tra on Oct 16 2020, 11:28 AM.

Details

Summary

We can not bitcast pointers across different address spaces, and VectorCombine
should be careful when it attempts to find the original source of the loaded
data.

Fixes an assertion trigger introduced by D81766

Diff Detail

Event Timeline

tra created this revision.Oct 16 2020, 11:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 16 2020, 11:28 AM
tra requested review of this revision.Oct 16 2020, 11:28 AM
spatel added inline comments.Oct 16 2020, 11:37 AM
llvm/test/Transforms/VectorCombine/AMDGPU/as-transition.ll
18

We still want to create a vector load rather than just bail out on the address-space mismatch?

tra added inline comments.Oct 16 2020, 12:20 PM
llvm/test/Transforms/VectorCombine/AMDGPU/as-transition.ll
18

I believe so. Vectorized load is still beneficial, assuming VectorCombine can do something useful w/o seeing through to the original pointer.
I don't know if it does, though. I can un-minimize this test case to do a full 4-element load and check.

I guess we may not be able to do it as often, but it should still be useful for AMDGPU and NVPTX where kernel arguments live in a special address space and the loads would be just one addrspacecast away from the original source.

Eventually we may want to have a special variant of stripPointerCasts() which would only strip casts until AS change boundary.

spatel accepted this revision.Oct 16 2020, 1:13 PM

LGTM - I don't have any experience with the target-specific perf, but this prevents a crash.

This revision is now accepted and ready to land.Oct 16 2020, 1:13 PM
This revision was automatically updated to reflect the committed changes.

Thanks a lot for fixing this.