[PATCH 2/3] SelectionDAG: Add KnownBits and SignBits computation for EXTRACT_ELEMENT

[Mehdi Amini]

Hi,


> On Jan 13, 2015, at 8:36 AM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
> 
> On Tue, 2014-11-25 at 19:08 -0500, Jan Vesely wrote:
>> On Tue, 2014-11-25 at 17:37 -0500, Matt Arsenault wrote:
>>>> On Nov 18, 2014, at 11:06 AM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
>>>> 
>>>> v2: use getZExtValue
>>>>   add missing break
>>>>   codestyle
>>>> 
>>>> Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu>
>>>> ---
>>>> lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 22 ++++++++++++++++++++++
>>>> 1 file changed, 22 insertions(+)
>>>> 
>>>> diff --git a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>>>> index 33620e7..09e0239 100644
>>>> --- a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>>>> +++ b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>>>> @@ -2323,6 +2323,19 @@ void SelectionDAG::computeKnownBits(SDValue Op, APInt &KnownZero,
>>>>    KnownZero = APInt::getHighBitsSet(BitWidth, Leaders);
>>>>    break;
>>>>  }
>>>> +  case ISD::EXTRACT_ELEMENT: {
>>>> +    computeKnownBits(Op.getOperand(0), KnownZero, KnownOne, Depth+1);
>>>> +    const unsigned Index =
>>>> +      cast<ConstantSDNode>(Op.getOperand(1))->getZExtValue();
>>>> +    const unsigned BitWidth = Op.getValueType().getSizeInBits();
>>>> +
>>>> +    KnownZero = KnownZero.getHiBits(KnownZero.getBitWidth() - Index * BitWidth);
>>>> +    KnownOne = KnownOne.getHiBits(KnownOne.getBitWidth() - Index * BitWidth);
>>>> +
>>>> +    KnownZero = KnownZero.trunc(BitWidth);
>>>> +    KnownOne = KnownOne.trunc(BitWidth);
>>>> +    break;
>>>> +  }
>>>>  case ISD::FrameIndex:
>>>>  case ISD::TargetFrameIndex:
>>>>    if (unsigned Align = InferPtrAlignment(Op)) {
>>>> @@ -2522,6 +2535,15 @@ unsigned SelectionDAG::ComputeNumSignBits(SDValue Op, unsigned Depth) const{
>>>>    // FIXME: it's tricky to do anything useful for this, but it is an important
>>>>    // case for targets like X86.
>>>>    break;
>>>> +  case ISD::EXTRACT_ELEMENT: {
>>>> +    const int KnownSign = ComputeNumSignBits(Op.getOperand(0), Depth+1);
>>>> +    const int BitWidth = Op.getValueType().getSizeInBits();
>>>> +    const int Items =
>>>> +      Op.getOperand(0).getValueType().getSizeInBits() / BitWidth;
>>>> +    const int rIndex = Items - 1 -
>>>> +      cast<ConstantSDNode>(Op.getOperand(1))->getZExtValue();
>>>> +    return std::max(std::min(KnownSign - rIndex * BitWidth, BitWidth), 0);
>>>> +  }
>>>>  }
>>>> 
>>>>  // If we are looking at the loaded value of the SDNode.
>>>> -- 
>>>> 1.9.3
>>>> 
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
>>> 
>>> These need tests. Do these really work? I remember trying to do this at
>>> some point, but then gave up because computeKnownBits doesn’t
>>> understand vectors very well
>> 
>> the tests are included in 3/3. this patch is needed so reduction from
>> i64->i32 can than be further turned into i24. Without this patch the
>> reduction stops at i32 and we'd need another check to do i64 to i24.
> 
> Ping
> 
> was that answer not good enough, or do I need to bug someone else to
> review this patch?


I’d rather see a one line comment in front of each case.

Thanks,

Mehdi


> 
> jan
> 
>> 
>>> 
>>> -Matt
>> 
> 
> -- 
> Jan Vesely <jan.vesely at rutgers.edu>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits