This is an archive of the discontinued LLVM Phabricator instance.

[OpenCL] Refactor out ReadPipe/WritePipe
ClosedPublic

Authored by joey on Nov 23 2016, 7:39 AM.

Details

Reviewers
yaron.keren
Summary

This patch to keep the pipe access qualifier inside PipeType itself looks cleaner overall than my previous approach.

Yaron had a post-commit comment, which lead me to re-think the original code.

Diff Detail

Repository
rL LLVM

Event Timeline

joey updated this revision to Diff 79079.Nov 23 2016, 7:39 AM
joey retitled this revision from to [OpenCL] Refactor out ReadPipe/WritePipe.
joey updated this object.
joey added reviewers: yaron.keren, bader.
joey set the repository for this revision to rL LLVM.
joey added a subscriber: cfe-commits.
bader removed a reviewer: bader.Nov 24 2016, 1:52 AM
bader added a subscriber: bader.
svenvh added a subscriber: svenvh.Nov 25 2016, 1:25 AM
yaron.keren added inline comments.Nov 26 2016, 12:31 PM
lib/AST/ASTContext.cpp
8258

I'm not too familar how PipeTypes should be merged, is it on purpose the process is not commutative,

ReadPipeType merged with WritePipeType is ReadPipeType

WritePipeType merged with ReadPipeType is WritePipeType

joey updated this revision to Diff 79399.Nov 28 2016, 7:04 AM

Pipe types cannot be merged by ASTContext::mergeTypes.

yaron.keren accepted this revision.Nov 29 2016, 12:35 PM
yaron.keren edited edge metadata.

LGTM after fixing the inline comment

lib/Serialization/ASTReader.cpp
5804
return Context,getPipeType(ElementType, ReadOnly)
This revision is now accepted and ready to land.Nov 29 2016, 12:35 PM
joey closed this revision.Dec 1 2016, 3:41 AM

Committed in r288332. Thanks Yaron!