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

Evan Cheng evan.cheng at apple.com
Wed Dec 4 10:00:46 PST 2013


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> 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
>>> 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
>> 
> 
> <pr17975.diff>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131204/a015d89e/attachment.html>


More information about the llvm-commits mailing list