Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

[C++4OpenCL] Fix initialization of __constant constructors without arguments

Authored by olestrohm on May 11 2021, 9:00 AM.



This fixes the implicit initialization that uses constructors in the __constant address space without arguments.

It also adds several tests to make sure that constructors in other address spaces also work, since these have not yet been tested.

Diff Detail

Event Timeline

olestrohm created this revision.May 11 2021, 9:00 AM
olestrohm requested review of this revision.May 11 2021, 9:00 AM
Anastasia added inline comments.May 12 2021, 5:11 AM

ctor -> Ctor


Btw, even though your change doesn't modify this I would suggest to add a CodeGen test that checks the initialization of objects in IR is adequate.


We seem to be missing the coverage with __constant and = default.


I find the wording of the error a bit odd, but it is standard C++ though...

olestrohm added inline comments.May 12 2021, 7:06 AM

This is because of the struct W below, you can't default a constexpr constructor, but __constant constructors need to be constexpr. Though it is currently possible to create them, they just can't actually be used. Or maybe just combine Z and W?

Anastasia added inline comments.May 12 2021, 8:21 AM

I see makes sense. I think it's ok to have separate structs. You could add a comment though to explain what you aim to test in each?

olestrohm updated this revision to Diff 345436.May 14 2021, 7:42 AM

Added a codegen test and clarified what some parts are testing.

Anastasia accepted this revision.May 14 2021, 7:54 AM

LGTM! Thanks!


FYI, I would remove , align 4 to simplify testing.

This revision is now accepted and ready to land.May 14 2021, 7:54 AM
olestrohm updated this revision to Diff 345845.May 17 2021, 5:44 AM

Relaxed the checks in the codegen test because of failures on platforms with slightly different IR being generated.

LGTM! Thanks!