rjmccall added a reviewer: Prazek.
rjmccall added a comment.
In https://reviews.llvm.org/D53295#1271590, @Anastasia wrote:
> 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.
================
Comment at: lib/CodeGen/CGBlocks.cpp:1318
+ CGM.getModule().getMDKindID("invariant.load"),
+ llvm::MDNode::get(getLLVMContext(), None));
+
----------------
yaxunl wrote:
> rjmccall wrote:
> > 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.
> 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.
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 `launder`s 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?
================
Comment at: lib/CodeGen/CGBlocks.cpp:980
+ auto storeField = [&](llvm::Value *value, unsigned index, CharUnits offset,
+ const Twine &name, bool IsInvariant) {
+ auto *ST = Builder.CreateStore(value, projectField(index, offset, name));
----------------
Please match the capitalization of local variable names used in the surrounding
code.
================
Comment at: test/CodeGenOpenCL/blocks-indirect-call.cl:11
+// NOOPT: load {{.*}}%[[INV]]{{.*}}, !invariant.group
+// ToDo: Fix LLVM optimizations to lower indirect function call
+// OPT: %[[INV:.*]] = phi {{.*}}[ @__blockInLoopCondition_block_invoke, %entry
]
----------------
I don't know what this TODO means.
https://reviews.llvm.org/D53295
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
