Page MenuHomePhabricator

[PR41674] [OpenCL] Fix initialisation of this via pointer
ClosedPublic

Authored by kpet on Apr 30 2019, 9:29 AM.

Details

Summary

When the expression used to initialise this has a pointer type,
check the address space of the pointee type instead of the pointer
type to decide whether an address space cast is required.
It is the pointee type that carries the address space qualifier.

Diff Detail

Repository
rC Clang

Event Timeline

kpet created this revision.Apr 30 2019, 9:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2019, 9:29 AM

I would suggest an IR test, especially since what you want to achieve is addrspacecast in IR output. We use AST dump only if there is no other good way to test something. In fact we already have a test with similar logic that tests various OpenCL address spaces: test/CodeGenOpenCLCXX/addrspace-of-this.cl. I think it should be reasonable enough to extend it.

kpet added a comment.May 1 2019, 10:18 AM

Ok, I'll replace the test with an IR-level test.

kpet updated this revision to Diff 197617.May 1 2019, 12:49 PM

Replaced the AST dump test with an IR test.

Anastasia added inline comments.May 2 2019, 3:27 AM
test/CodeGenOpenCLCXX/addrspace-of-this.cl
94

Ok I think we should regex name of call registers or may be even prevent checking it if it's not required. The same applies to other places btw.

136

Shouldn't %call5 just be [[CALL]]? If not I see little value in checking the name...

kpet added a comment.May 2 2019, 4:33 AM

Most of these are pre-existing issues with the tests but I agree they're worth fixing. I'll update the tests.

kpet updated this revision to Diff 197749.May 2 2019, 4:48 AM

Clean up the tests.

Anastasia accepted this revision.May 2 2019, 8:18 AM

LGTM! Thanks!

This revision is now accepted and ready to land.May 2 2019, 8:18 AM
Closed by commit rC359798: [OpenCL] Fix initialisation of this via pointer. (authored by stulova, committed by ). · Explain WhyMay 2 2019, 9:08 AM
This revision was automatically updated to reflect the committed changes.