[PATCH] D47423: Simplify recursive launder.invariant.group and strip

Piotr Padlewski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 3 10:28:34 PDT 2018


Prazek added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1220-1221
+    Result = IC.Builder.CreateLaunderInvariantGroup(StrippedInvariantGroupsArg);
+  else if (II.getIntrinsicID() == Intrinsic::strip_invariant_group)
+    Result = IC.Builder.CreateStripInvariantGroup(StrippedInvariantGroupsArg);
+  else
----------------
amharc wrote:
> Prazek wrote:
> > rsmith wrote:
> > > I think we can do a little better for the `strip(strip(p))` case. If we have:
> > > 
> > > ```
> > > T *p = ...;
> > > T *q = strip(p);
> > > T *r = strip(q);
> > > ```
> > > 
> > > ... it would seem preferable to rewrite that as:
> > > 
> > > ```
> > > T *r = q;
> > > ```
> > > 
> > > rather than as:
> > > 
> > > ```
> > > T *r = strip(p);
> > > ```
> > > 
> > > (Will GVN rewrite the latter as the former anyway?)
> > No, none of the optimization can remove strips. I don't think it is legal in our model and I don't see it how it could work
> Isn't `strip` supposed to be pure, and hence two calls to `strip(p)` should be CSE'd by GVN?
Oh sorry Richard, I thought you mean 
  T *r = p; 

The outcome of this transformation will be

T *q = strip(p);
T* r = strip(p);

And then GVN will see that strip(p) is always the same and will do

T* q = strip(p);
T* r = q;

So that is exactly what you said.


Repository:
  rL LLVM

https://reviews.llvm.org/D47423





More information about the llvm-commits mailing list