llvm dev - Jul 2009 - [LLVMdev] Vector logic regression in r73431

Hi all,
Hi Eli,

No, that appears to be something unrelated. I'm currently using revision
75246, while that patch only seems to apply to some later revision.

Anyway, I actually located the real bug. Right at the end of
InstCombiner::SimplifyDemandedUseBits, there's this piece of code:

  // If the client is only demanding bits that we know, return the known
  // constant.
  if ((DemandedMask & (RHSKnownZero|RHSKnownOne)) == DemandedMask) {
    Constant *C = Context->getConstantInt(RHSKnownOne);
    if (isa<PointerType>(V->getType()))
      C = Context->getConstantExprIntToPtr(C, V->getType());
    return C;
  }
  return false;
}

Note that C is a scalar integer, and so when V is actually a vector the type
isn't preserved.

I'm not entirely sure how this function is supposed to work with vectors
though. DemandedMask, KnownOne and KnownZero are APInt's (scalars) the size
of an element of the vector. So when, as the comment describes, only known
bits are demanded, does that apply to just one element of the vector or all?

Thanks,

Nicolas


-----Original Message-----
From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On
Behalf Of Eli Friedman
Sent: woensdag 29 juli 2009 20:44
To: LLVM Developers Mailing List
Subject: Re: [LLVMdev] Vector logic regression in r73431

On Wed, Jul 29, 2009 at 3:45 AM, Nicolas Capens<nicolas at capens.net>
wrote:
> So could anyone who knows the ins and outs of this code have a look at 
> how to make it handle vectors correctly? Or if that's not an option 
> right now, please revert the broken optimizations. Note that there 
> might be more things affected than visitAnd, visitOr and vistXor, I've 
> only been able to identify these so far with little knowledge of the 
> actual code. I currently don't have a reduced testcase, but if really
necessary I can try to extract one.

Does the attached help?

-Eli

On Thu, Jul 30, 2009 at 5:57 AM, Nicolas Capens<nicolas at capens.net>
wrote:
> No, that appears to be something unrelated. I'm currently using
revision
> 75246, while that patch only seems to apply to some later revision.
I don't see the connection... anyway, I can't easily help you with an
old revision.
> Anyway, I actually located the real bug. Right at the end of
> InstCombiner::SimplifyDemandedUseBits, there's this piece of code:
>
>  // If the client is only demanding bits that we know, return the known
>  // constant.
>  if ((DemandedMask & (RHSKnownZero|RHSKnownOne)) == DemandedMask) {
>    Constant *C = Context->getConstantInt(RHSKnownOne);
>    if (isa<PointerType>(V->getType()))
>      C = Context->getConstantExprIntToPtr(C, V->getType());
>    return C;
>  }
>  return false;
> }
>
> Note that C is a scalar integer, and so when V is actually a vector the
type
> isn't preserved.
Right... my patch fixes that, I think.
> I'm not entirely sure how this function is supposed to work with
vectors
> though. DemandedMask, KnownOne and KnownZero are APInt's (scalars) the
size
> of an element of the vector. So when, as the comment describes, only known
> bits are demanded, does that apply to just one element of the vector or
all?
It applies to all elements of the vector.

-Eli

Oh...

Now I see. Your patch does address this issue. It seemed so unrelated at
first, and I didn't look closer at it after I was unable to apply it to my
(slightly) older revision. Sorry about that.

Anyway, I can confirm that your patch fixed the asserts and crashes I was
seeing. So as far as I'm concerned go ahead and commit it if you haven't
done so already.

Thanks!

Nicolas


-----Original Message-----
From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On
Behalf Of Eli Friedman
Sent: donderdag 30 juli 2009 18:23
To: LLVM Developers Mailing List
Subject: Re: [LLVMdev] Vector logic regression in r73431

On Thu, Jul 30, 2009 at 5:57 AM, Nicolas Capens<nicolas at capens.net>
wrote:
> No, that appears to be something unrelated. I'm currently using
revision
> 75246, while that patch only seems to apply to some later revision.
I don't see the connection... anyway, I can't easily help you with an
old revision.
> Anyway, I actually located the real bug. Right at the end of
> InstCombiner::SimplifyDemandedUseBits, there's this piece of code:
>
>  // If the client is only demanding bits that we know, return the known
>  // constant.
>  if ((DemandedMask & (RHSKnownZero|RHSKnownOne)) == DemandedMask) {
>    Constant *C = Context->getConstantInt(RHSKnownOne);
>    if (isa<PointerType>(V->getType()))
>      C = Context->getConstantExprIntToPtr(C, V->getType());
>    return C;
>  }
>  return false;
> }
>
> Note that C is a scalar integer, and so when V is actually a vector the
type
> isn't preserved.
Right... my patch fixes that, I think.
> I'm not entirely sure how this function is supposed to work with
vectors
> though. DemandedMask, KnownOne and KnownZero are APInt's (scalars) the
size
> of an element of the vector. So when, as the comment describes, only known
> bits are demanded, does that apply to just one element of the vector or
all?

It applies to all elements of the vector.

-Eli

_______________________________________________
LLVM Developers mailing list
LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev