<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>I am not sure how this patch addresses the cost problem. You are still generating a new ZEXT instruction. Why is it better for all platforms ? Is it better for AMD’s GPU ? If it is better only for ARM and X86 then you need to put it in these backends. </div><div><br><div><div>On Jun 23, 2013, at 10:39 PM, "Liao, Michael" <<a href="mailto:michael.liao@intel.com">michael.liao@intel.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: 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;">Sorry, I don't why the system put that mail into junk box. I just check that about. In case you miss my reply. I put it here again.<br><br>Good point. I thought the original transformation is targeted to reduce overhead of shift in some scenarios when data types must be extended, e.g. 64-bit integer on 32-bit targets. It's painful to do that if all significant bits are required.<br><br>For 64-bit targets, reducing a 64-bit shift to a 32-bit one might have benefit as well. E.g., you may save one byte on x86-64 without encoding the W bit.<br><br>We could go even aggressively by folding (srl (anyextend x), c) to (zext (srl x, c)) to help generate efficient code on all targets as 'zext'<br>has better support. I verified it on x86-64 and ppc64 where the same code is generated before the bug is fixed. I checked aarch64 and mips64 as well but it seems those targets doesn't take the advantage that high bits are set to ZERO (I learned from public source that, with 32-bit instruction, on aarch64, the upper 32 bits of the destination register are set to ZERO and, on mips64, the upper 32 bits of the destination register will be sign extended and will be cleared for this case.) Please check the patch attached.<br><br>- michael<br><br><br>-----Original Message-----<br>From: <a href="mailto:llvm-commits-bounces@cs.uiuc.edu">llvm-commits-bounces@cs.uiuc.edu</a> [<a href="mailto:llvm-commits-bounces@cs.uiuc.edu">mailto:llvm-commits-bounces@cs.uiuc.edu</a>] On Behalf Of Duncan Sands<br>Sent: Sunday, June 23, 2013 5:49 AM<br>To: <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>Subject: Re: [llvm] r184575 - Fix PR16360<br><br>On 21/06/13 21:22, Nadav Rotem wrote:<br><blockquote type="cite">It is not clear to me why this transformation is profitable for all platforms.<br>You are adding an AND operation here. Should this optimization be<span class="Apple-converted-space"> </span><br>target specific maybe ?<br></blockquote><br>I made the same point and also mentioned a better way of generating the mask, but Michael seems to have ignored my email.<br><br>Ciao, Duncan.<br><br><blockquote type="cite"><br>On Jun 21, 2013, at 11:45 AM, Michael Liao <<a href="mailto:michael.liao@intel.com">michael.liao@intel.com</a><span class="Apple-converted-space"> </span><br><<a href="mailto:michael.liao@intel.com">mailto:michael.liao@intel.com</a>>> wrote:<br><br><blockquote type="cite">Author: hliao<br>Date: Fri Jun 21 13:45:27 2013<br>New Revision: 184575<br><br>URL:http://<a href="http://llvm.org/viewvc/llvm-project?rev=184575&view=rev">llvm.org/viewvc/llvm-project?rev=184575&view=rev</a><br>Log:<br>Fix PR16360<br><br>When (srl (anyextend x), c) is folded into (anyextend (srl x, c)),<span class="Apple-converted-space"> </span><br>the high bits are not cleared. Add 'and' to clear off them.<br><br><br>Added:<br> llvm/trunk/test/CodeGen/X86/pr16360.ll<br>Modified:<br> llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br><br>Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>URL:http://<a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/Select">llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/Select</a><br>ionDAG/DAGCombiner.cpp?rev=184575&r1=184574&r2=184575&view=diff<br>=====================================================================<br>=========<br>--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)<br>+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Fri Jun 21<span class="Apple-converted-space"> </span><br>+++ 13:45:27 2013<br>@@ -3915,8 +3915,7 @@ SDValue DAGCombiner::visitSRL(SDNode *N)<br> DAG.getConstant(~0ULL >> ShAmt, VT));<br> }<br><br>-<br>- // fold (srl (anyextend x), c) -> (anyextend (srl x, c))<br>+ // fold (srl (anyextend x), c) -> (and (anyextend (srl x, c)),<span class="Apple-converted-space"> </span><br>+ mask)<br> if (N1C && N0.getOpcode() == ISD::ANY_EXTEND) {<br> // Shifting in all undef bits?<br> EVT SmallVT = N0.getOperand(0).getValueType(); @@ -3929,7<span class="Apple-converted-space"> </span><br>+3928,10 @@ SDValue DAGCombiner::visitSRL(SDNode *N)<br> N0.getOperand(0),<br> DAG.getConstant(ShiftAmt, getShiftAmountTy(SmallVT)));<br> AddToWorkList(SmallShift.getNode());<br>- return DAG.getNode(ISD::ANY_EXTEND, SDLoc(N), VT, SmallShift);<br>+ APInt Mask = APInt::getAllOnesValue(VT.getSizeInBits()).lshr(ShiftAmt);<br>+ return DAG.getNode(ISD::AND, SDLoc(N), VT,<br>+ DAG.getNode(ISD::ANY_EXTEND, SDLoc(N), VT, SmallShift),<br>+ DAG.getConstant(Mask, VT));<br> }<br> }<br><br><br>Added: llvm/trunk/test/CodeGen/X86/pr16360.ll<br>URL:http://<a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/p">llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/p</a><br>r16360.ll?rev=184575&view=auto<span class="Apple-converted-space"> </span><br>=====================================================================<br>=========<br>--- llvm/trunk/test/CodeGen/X86/pr16360.ll (added)<br>+++ llvm/trunk/test/CodeGen/X86/pr16360.ll Fri Jun 21 13:45:27 2013<br>@@ -0,0 +1,16 @@<br>+; RUN: llc < %s -mtriple=i686-pc-linux | FileCheck %s<br>+<br>+define i64 @foo(i32 %sum) {<br>+entry:<br>+ %conv = sext i32 %sum to i64<br>+ %shr = lshr i64 %conv, 2<br>+ %or = or i64 4611686018360279040, %shr<br>+ ret i64 %or<br>+}<br>+<br>+; CHECK: foo<br>+; CHECK: shrl $2<br>+; CHECK: orl $-67108864<br>+; CHECK-NOT: movl $-1<br>+; CHECK: movl $1073741823<br>+; CHECK: ret<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a> <<a href="mailto:llvm-commits@cs.uiuc.edu">mailto:llvm-commits@cs.uiuc.edu</a>><span class="Apple-converted-space"> </span><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><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>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br><br></blockquote><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><span><0001-Revise-fix-to-PR16360-for-efficient-code-on-64-bit-t.patch></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>