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

[Bill Wendling]

I reverted r191049 and r191059 from 3.4.


On Wed, Dec 4, 2013 at 10:00 AM, Evan Cheng <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> 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
>
>
>
> _______________________________________________
> 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/20131205/904e61c5/attachment.html>