This is an archive of the discontinued LLVM Phabricator instance.

[OpenCL] Handle address space conversion while setting type alignment
ClosedPublic

Authored by Anastasia on Sep 13 2017, 6:26 AM.

Diff Detail

Event Timeline

Anastasia created this revision.Sep 13 2017, 6:26 AM
yaxunl added inline comments.Sep 13 2017, 1:59 PM
lib/CodeGen/CGExpr.cpp
957

Better assert that only CK_AddressSpaceConversion allows different addr spaces in target type.

Anastasia added inline comments.Sep 14 2017, 10:38 AM
lib/CodeGen/CGExpr.cpp
957

It seems for consistency then we would have to assert for both different ASes in non-addrspacecast case and equivalent ASes in the other case.

As condition becomes complicated then, I could as well split into either creating bitcast or addrspacecast explicitly at a cost of one simple check in the return statement but that would be in release build too.

Anastasia added inline comments.Sep 19 2017, 9:04 AM
lib/CodeGen/CGExpr.cpp
957

Sam, do you prefer an assert that will check both things or a runtime check that would make sure to build the right IR node here?

yaxunl added inline comments.Sep 20 2017, 12:12 PM
lib/CodeGen/CGExpr.cpp
957

splitting into bitcast and addrspacecast seems better.

Anastasia updated this revision to Diff 116345.Sep 22 2017, 7:02 AM

Created BitCast or AddrSpaceCast conditionally.

yaxunl accepted this revision.Sep 22 2017, 10:23 AM

LGTM. Thanks.

This revision is now accepted and ready to land.Sep 22 2017, 10:23 AM
bader added a subscriber: bader.Sep 22 2017, 11:01 AM
bader added inline comments.
test/CodeGenOpenCL/vectorLoadStore.cl
7

Can we remove this line?

14

Please, set SPIR target in clang options to make this check reliable.

Anastasia updated this revision to Diff 116548.Sep 25 2017, 7:09 AM

Addressed comments from Alexey.

bader accepted this revision.Sep 25 2017, 7:23 AM

LGTM. Thanks!

This revision was automatically updated to reflect the committed changes.

vectorLoadStore.cl is failing on our bots: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/8187, please check it out

Committed in r314304

vectorLoadStore.cl is failing on our bots: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/8187, please check it out

I will commit a fix in a bit.

Committed in r314317.