[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:25:46 PST 2013


Thanks!

On 05.12.2013 19:33, Bill Wendling wrote:
> I reverted r191049 and r191059 from 3.4.
>
>
> On Wed, Dec 4, 2013 at 10:00 AM, Evan Cheng <evan.cheng at apple.com
> <mailto:evan.cheng at apple.com>> 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 <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