[PATCH] Fix for PR17975 - wrong code for bit shifting integer promotion at -O1 and above on x86_64-linux-gnu

Kai Nacke kai.nacke at redstar.de
Sat Dec 7 05:28:29 PST 2013


Hi Evan!

Thanks for the comments. I address them in a later version of my patch.
Bill has already reverted my commits in the 3.4 branch so the release is 
safe.

Regards,
Kai

On 04.12.2013 19:00, Evan Cheng wrote:
> This patch is hard to read. For example:
>
> -        if (HasROTRWithLArg || HasROTLWithLArg) {
> +        ConstantSDNode *CN;
> +        if (Use->getOpcode() == ISD::AND &&
> +            (CN = dyn_cast<ConstantSDNode>(Use->getOperand(1))) &&
> +            CN->getZExtValue()+1 == (1ULL << LArgVT.getSizeInBits())
> +            && (HasROTRWithLArg || HasROTLWithLArg)) {
>
>
> I would factor it out and match the earlier comment better. Perhaps
> something like:
>
> +        if (Use->getOpcode() == ISD::AND &&
> +            && (HasROTRWithLArg || HasROTLWithLArg)) {
> +           ConstantSDNode *CN =
> dyn_cast<ConstantSDNode>(Use->getOperand(1));
> +           // More comments here
> +           if (CN &&
> +              CN->getZExtValue()+1 == (1ULL << LArgVT.getSizeInBits())
>
> This is also faster since the code delay the dyn_cast until cheaper
> conditions are checked.
>
> Your description mentions "truncate". However, this is a AND masking,
> which is not quite the same.
>
> As for 3.4, perhaps the safest thing to do now is to revert your patch
> r191059.
>
> Evan
>
> On Dec 4, 2013, at 2:27 AM, Kai Nacke <kai.nacke at redstar.de
> <mailto:kai.nacke at redstar.de>> wrote:
>
>> Ping^2.
>>
>> This is a regression from LLVM 3.3 and should IMHO fixed - either by
>> reverting my commit or by applying the attached fix.
>>
>> Regards,
>> Kai
>>
>> On 02.12.2013 08:13, Kai Nacke wrote:
>>> Ping.
>>>
>>> On 25.11.2013 08:12, Kai Nacke wrote:
>>>> Hi all!
>>>>
>>>> With my commit r191059 I introduced new patterns to match rotates. But I
>>>> forgot to check for the required TRUNCATE after the rotate. The missing
>>>> check produces wrong code as documented in PR17975.
>>>>
>>>> The attached patch fixes the problem by adding the missing check. It
>>>> also extends the test to include patterns which must not produce a
>>>> rotate. Please review.
>>>>
>>>> @Owen Anderson, Bill Wendling
>>>> If approved then this patch needs to be committed to the 3.4 branch. If
>>>> this seems to be to risky then my commit r191059 must be reverted on the
>>>> 3.4 branch in order to fix the wrong code generation.
>>>>
>>>> Regards,
>>>> Kai
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
>> <pr17975.diff>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>





More information about the llvm-commits mailing list