Page MenuHomePhabricator

Mark store and load of block invoke function as invariant.group
Needs ReviewPublic

Authored by yaxunl on Oct 15 2018, 11:11 AM.

Details

Summary

block invoke function once is stored in block literal, it will not change.
Therefore mark store and load of block invoke function as invariant.group.

This will facilitate lowering of indirect function calls to direct function
calls, which is important for OpenCL since OpenCL does not require
function pointer support.

Diff Detail

Event Timeline

yaxunl created this revision.Oct 15 2018, 11:11 AM
rjmccall added inline comments.Oct 15 2018, 12:03 PM
lib/CodeGen/CGBlocks.cpp
1324

OpenCL blocks are still potentially function-local, right? I don't think you're allowed to put invariant.load on something that's visibly initialized, even if it's visibly initialized to the same thing every time. The problem is that invariant.load could allow the load to be hoisted above the initialization.

If you can solve that problem, you can make this non-OpenCL-specific.

yaxunl added inline comments.Oct 16 2018, 4:22 AM
lib/CodeGen/CGBlocks.cpp
1324

It seems that invariant.load implies the pointer is invariant in the whole module, disregarding the store to it, which is not suitable for this case.

In this case what I need is that after the first store to the pointer, every load does not change.

It seems invariant.group can be used for this.

yaxunl updated this revision to Diff 169864.Oct 16 2018, 11:14 AM
yaxunl retitled this revision from [OpenCL] Mark load of block invoke function as invariant to Mark store and load of block invoke function as invariant.group.
yaxunl edited the summary of this revision. (Show Details)

mark store and load of block invoke function as invariant.group.

Btw, blocks w/o captures are already optimized into regular calls?

Btw, blocks w/o captures are already optimized into regular calls?

That's a very easy optimization for the optimizer to do because the global can be marked constant.

lib/CodeGen/CGBlocks.cpp
980

Please match the capitalization of local variable names used in the surrounding code.

1324

Hmm. I guess invariant.group does what you need because LLVM will probably unify the GEPs to extract the function-pointer field if they appear in the same function, and there are no launderings of that pointer.

CC'ing Piotr to get his opinion about whether this is a correct use of invariant.group. Piotr, we have a struct-typed alloca that we initialize in a probably-dominating position. A pointer to the struct can be passed off to other contexts which we can't necessarily analyze, blocking normal optimization; however, we know that this struct can (mostly) not be legally modified after initialization. We're marking one of the initializing stores as well as the load of the corresponding field when it's used. These two places are completely separate and will perform their own GEPs, but because we don't emit any launders on the struct, LLVM will likely unify the GEPs if they appear in the same function, allowing invariant.group-based optimizations to apply. Does this seem reasonable?

test/CodeGenOpenCL/blocks-indirect-call.cl
11

I don't know what this TODO means.

Prazek added inline comments.Fri, Jun 14, 5:40 PM
lib/CodeGen/CGBlocks.cpp
1324

Yep, seems totally reasonable. Sorry for such a late response, I just have open reviews and saw that.