[llvm] r331792 - DAG: Use correct shift width type

[Friedman, Eli via llvm-commits]

On 5/14/2018 8:47 AM, Matt Arsenault wrote:
>
>> On May 9, 2018, at 00:46, Friedman, Eli via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>> On 5/8/2018 11:43 AM, Matt Arsenault via llvm-commits wrote:
>>> Author: arsenm
>>> Date: Tue May  8 11:43:05 2018
>>> New Revision: 331792
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=331792&view=rev
>>> Log:
>>> DAG: Use correct shift width type
>>>
>>> Modified:
>>>      llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
>>>
>>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp?rev=331792&r1=331791&r2=331792&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp Tue May  8 11:43:05 2018
>>> @@ -1076,7 +1076,7 @@ SDValue DAGTypeLegalizer::JoinIntegers(S
>>>     Hi = DAG.getNode(ISD::ANY_EXTEND, dlHi, NVT, Hi);
>>>     Hi = DAG.getNode(ISD::SHL, dlHi, NVT, Hi,
>>>                      DAG.getConstant(LVT.getSizeInBits(), dlHi,
>>> -                                   TLI.getPointerTy(DAG.getDataLayout())));
>>> +                                   TLI.getShiftAmountTy(NVT, DAG.getDataLayout())));
>> Should this be "TLI.getShiftAmountTy(NVT, DAG.getDataLayout(), false)"?
>>
>> -Eli
> I’m not sure. I don’t really understand what the point of that option is. This is during type legalization, so maybe it should be producing a legal shift type?

The general problem is that getScalarShiftAmountTy on some targets 
unconditionally returns a type smaller than i32, so it's possible to 
accidentally truncate the shift amount when you're dealing with very big 
integers (since LLVM integer types can have up to 2^23 bits).

getShiftAmountTy is a wrapper which returns the getScalarShiftAmountTy 
if you pass "true", and a wider type if you pass "false".  You need the 
wider type before and during type legalization, so you need to pass 
"false" here.  See https://reviews.llvm.org/D43449 for more about the 
flag specifically.

Yes, this is confusing; we should try to implement a better solution.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project