<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">This patch is hard to read. For example:<div><br></div><div><div>-        if (HasROTRWithLArg || HasROTLWithLArg) {</div><div>+        ConstantSDNode *CN;</div><div>+        if (Use->getOpcode() == ISD::AND &&</div><div>+            (CN = dyn_cast<ConstantSDNode>(Use->getOperand(1))) &&</div><div>+            CN->getZExtValue()+1 == (1ULL << LArgVT.getSizeInBits())</div><div>+            && (HasROTRWithLArg || HasROTLWithLArg)) {</div><div><br></div><div><br></div><div>I would factor it out and match the earlier comment better. Perhaps something like:</div><div><br></div><div><div>+        if (Use->getOpcode() == ISD::AND &&</div><div>+            && (HasROTRWithLArg || HasROTLWithLArg)) {</div></div><div>+           ConstantSDNode *CN = dyn_cast<ConstantSDNode>(Use->getOperand(1));</div><div>+           // More comments here</div><div>+           if (CN &&</div><div><div>+              CN->getZExtValue()+1 == (1ULL << LArgVT.getSizeInBits())</div></div><div><br></div><div>This is also faster since the code delay the dyn_cast until cheaper conditions are checked.</div><div><br></div><div>Your description mentions "truncate". However, this is a AND masking, which is not quite the same.</div><div><br></div><div>As for 3.4, perhaps the safest thing to do now is to revert your patch r191059.</div><div><br></div><div>Evan</div><div><br></div><div><div>On Dec 4, 2013, at 2:27 AM, Kai Nacke <<a href="mailto:kai.nacke@redstar.de">kai.nacke@redstar.de</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Ping^2.<br><br>This is a regression from LLVM 3.3 and should IMHO fixed - either by reverting my commit or by applying the attached fix.<br><br>Regards,<br>Kai<br><br>On 02.12.2013 08:13, Kai Nacke wrote:<br><blockquote type="cite">Ping.<br><br>On 25.11.2013 08:12, Kai Nacke wrote:<br><blockquote type="cite">Hi all!<br><br>With my commit r191059 I introduced new patterns to match rotates. But I<br>forgot to check for the required TRUNCATE after the rotate. The missing<br>check produces wrong code as documented in PR17975.<br><br>The attached patch fixes the problem by adding the missing check. It<br>also extends the test to include patterns which must not produce a<br>rotate. Please review.<br><br>@Owen Anderson, Bill Wendling<br>If approved then this patch needs to be committed to the 3.4 branch. If<br>this seems to be to risky then my commit r191059 must be reverted on the<br>3.4 branch in order to fix the wrong code generation.<br><br>Regards,<br>Kai<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br><br></blockquote><br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br><br></blockquote><br><span><pr17975.diff></span>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div><br></div></body></html>