[PATCH] D44063: [InstCombine] Don't blow up in foldICmpWithCastAndCast on vector icmp instructions.

[Anna Thomas via Phabricator via llvm-commits]

anna accepted this revision.
anna added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:3411
   // integer type is the same size as the pointer type.
+  const auto& CompatibleSizes = [&](Type* SrcTy, Type* DestTy) -> bool {
+    if (isa<VectorType>(SrcTy)) {
----------------
dneilson wrote:
> anna wrote:
> > dneilson wrote:
> > > anna wrote:
> > > > Instead of capturing everything, let's capture just `DL`.
> > > This is not capturing everything, it is only capturing the this pointer.
> > > 
> > > Per: http://en.cppreference.com/w/cpp/language/lambda
> > > 
> > > ```
> > > [&] captures all automatic variables **used in the body** of the lambda by reference and current object by reference if exists (emphasis mine).
> > > ```
> > > 
> > > There are no automatics used in the body of this lambda, so only the current object reference (i.e. the this pointer) is captured.
> > > 
> > > I could change [&] to [this] to make it more explicit, if desired...
> > But why do you need to capture `this`? Basically, this should do right:
> > ```
> > const auto& CompatibleSizes = [&DL](Type* SrcTy, Type* DestTy) -> bool {
> > ....
> > ```
> > It makes it clearer that the only capture required is DL which is used in the lambda.
> > 
> >  
> Non-obvious detail: DL is a member of the InstCombiner class, and not an automatic.
> 
> You can't capture members. Can only capture reference to current object (i.e. this), and automatics.
ah I didn't see that. 


Repository:
  rL LLVM

https://reviews.llvm.org/D44063