[LLVMdev] ComputeMaskedBits Bug

[Nick Lewycky]

David Greene wrote:
> On Friday 18 July 2008 00:36, Nick Lewycky wrote:
>> David Greene wrote:
>>> Is my analysis correct?  If so, is the PHI code the culprit (for not
>>> returning the min of the KnownZero bits) or is the Shl code the culprit
>>> (for not paying attention to the Mask passed in (it right shifts it)?
>> I think your analysis is correct, and that Shl -- and many of the other
>> operations (AShr, LShr, SExt, Add?, Call?) -- should be modified to
>> always respect Mask.
> 
> After thinking about this some more, I'm not so sure.  The Shl really DOES
> result in six low zero bits, so it should return that.  That PHI node has
> semantics that it should take care of itself (taking the min of the known
> zero and one bits).
> 
> It seems a little brittle to me to have the PHI pass its semantics down
> to children via the Mask.  It should just do what it knows is right in the
> first place.
> 
> That said, there are many places that don't respect the Mask.  Closer
> reading of the comment leads me to believe the Mask is simply a 
> time-saving device, not a correctness-enforcing mechanism.

That's fine, but if you fix it that way, please audit 
InstructionCombiner SimplifyDemandedBits, which I believe has the same bug.

> I've fixed the PHI analysis to do the min in our code and it fixes the
> testcase I was working on.  Doing a min like this would also allow us
> to have PHI nodes compute known zero and one bits even when there
> isn't a recurrence.

Great! Did you commit a patch for this?

Nick