<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 5, 2015, at 12:27 PM, Craig Topper <<a href="mailto:craig.topper@gmail.com" class="">craig.topper@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">No because they just end up getting ignored in the convertToThreeAddress function in X86InstrInfo.cpp. This change just stops the TwoAddressInstructionPass from even trying to call convertoToThreeAddress for these instructions. </div></div></blockquote><div><br class=""></div><div>I see, thanks for the clarification.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class="">Perhaps we should have an assertion in convertToThreeAddress in the default case of the switch to catch more of these mistagged instructions.</div></div></blockquote><div><br class=""></div><div>I have not checked what does the documentation say for the related target hook, but I guess it is valid to return an “invalid” opcode. Think instruction that is convertible to three address but only for some inputs.</div><div>The bottom line is, I do not think the assertion will be appropriate, but we may consider adding a debug output to express that the conversion did not happen (if that is not already here) and perhaps some internal option to turn that into a crash for debugging purposes.</div><div>What do you think?</div><div><br class=""></div><div>Thanks,</div><div>Q.</div><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Mon, Jan 5, 2015 at 12:18 PM, Quentin Colombet <span dir="ltr" class=""><<a href="mailto:qcolombet@apple.com" target="_blank" class="">qcolombet@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Craig,<br class="">
<br class="">
Would it be possible to have a test case for this?<br class="">
<br class="">
Thanks,<br class="">
-Quentin<br class="">
<br class="">
> On Dec 29, 2014, at 8:25 AM, Craig Topper <<a href="mailto:craig.topper@gmail.com" class="">craig.topper@gmail.com</a>> wrote:<br class="">
><br class="">
> Author: ctopper<br class="">
> Date: Mon Dec 29 10:25:26 2014<br class="">
> New Revision: 224940<br class="">
><br class="">
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=224940&view=rev" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=224940&view=rev</a><br class="">
> Log:<br class="">
> [X86] Fix some cases where some 8-bit instructions were marked as being convertible to three address instructions, but aren't really.<br class="">
><br class="">
> Modified:<br class="">
> llvm/trunk/lib/Target/X86/X86InstrArithmetic.td<br class="">
><br class="">
> Modified: llvm/trunk/lib/Target/X86/X86InstrArithmetic.td<br class="">
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrArithmetic.td?rev=224940&r1=224939&r2=224940&view=diff" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrArithmetic.td?rev=224940&r1=224939&r2=224940&view=diff</a><br class="">
> ==============================================================================<br class="">
> --- llvm/trunk/lib/Target/X86/X86InstrArithmetic.td (original)<br class="">
> +++ llvm/trunk/lib/Target/X86/X86InstrArithmetic.td Mon Dec 29 10:25:26 2014<br class="">
> @@ -999,12 +999,13 @@ multiclass ArithBinOp_RF<bits<8> BaseOpc<br class="">
> bit CommutableRR, bit ConvertibleToThreeAddress> {<br class="">
> let Defs = [EFLAGS] in {<br class="">
> let Constraints = "$src1 = $dst" in {<br class="">
> - let isCommutable = CommutableRR,<br class="">
> - isConvertibleToThreeAddress = ConvertibleToThreeAddress in {<br class="">
> + let isCommutable = CommutableRR in {<br class="">
> def NAME#8rr : BinOpRR_RF<BaseOpc, mnemonic, Xi8 , opnodeflag>;<br class="">
> - def NAME#16rr : BinOpRR_RF<BaseOpc, mnemonic, Xi16, opnodeflag>;<br class="">
> - def NAME#32rr : BinOpRR_RF<BaseOpc, mnemonic, Xi32, opnodeflag>;<br class="">
> - def NAME#64rr : BinOpRR_RF<BaseOpc, mnemonic, Xi64, opnodeflag>;<br class="">
> + let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {<br class="">
> + def NAME#16rr : BinOpRR_RF<BaseOpc, mnemonic, Xi16, opnodeflag>;<br class="">
> + def NAME#32rr : BinOpRR_RF<BaseOpc, mnemonic, Xi32, opnodeflag>;<br class="">
> + def NAME#64rr : BinOpRR_RF<BaseOpc, mnemonic, Xi64, opnodeflag>;<br class="">
> + } // isConvertibleToThreeAddress<br class="">
> } // isCommutable<br class="">
><br class="">
> def NAME#8rr_REV : BinOpRR_Rev<BaseOpc2, mnemonic, Xi8>;<br class="">
> @@ -1017,6 +1018,8 @@ multiclass ArithBinOp_RF<bits<8> BaseOpc<br class="">
> def NAME#32rm : BinOpRM_RF<BaseOpc2, mnemonic, Xi32, opnodeflag>;<br class="">
> def NAME#64rm : BinOpRM_RF<BaseOpc2, mnemonic, Xi64, opnodeflag>;<br class="">
><br class="">
> + def NAME#8ri : BinOpRI_RF<0x80, mnemonic, Xi8 , opnodeflag, RegMRM>;<br class="">
> +<br class="">
> let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {<br class="">
> // NOTE: These are order specific, we want the ri8 forms to be listed<br class="">
> // first so that they are slightly preferred to the ri forms.<br class="">
> @@ -1024,7 +1027,6 @@ multiclass ArithBinOp_RF<bits<8> BaseOpc<br class="">
> def NAME#32ri8 : BinOpRI8_RF<0x82, mnemonic, Xi32, opnodeflag, RegMRM>;<br class="">
> def NAME#64ri8 : BinOpRI8_RF<0x82, mnemonic, Xi64, opnodeflag, RegMRM>;<br class="">
><br class="">
> - def NAME#8ri : BinOpRI_RF<0x80, mnemonic, Xi8 , opnodeflag, RegMRM>;<br class="">
> def NAME#16ri : BinOpRI_RF<0x80, mnemonic, Xi16, opnodeflag, RegMRM>;<br class="">
> def NAME#32ri : BinOpRI_RF<0x80, mnemonic, Xi32, opnodeflag, RegMRM>;<br class="">
> def NAME#64ri32: BinOpRI_RF<0x80, mnemonic, Xi64, opnodeflag, RegMRM>;<br class="">
> @@ -1080,12 +1082,13 @@ multiclass ArithBinOp_RFF<bits<8> BaseOp<br class="">
> bit ConvertibleToThreeAddress> {<br class="">
> let Uses = [EFLAGS], Defs = [EFLAGS] in {<br class="">
> let Constraints = "$src1 = $dst" in {<br class="">
> - let isCommutable = CommutableRR,<br class="">
> - isConvertibleToThreeAddress = ConvertibleToThreeAddress in {<br class="">
> + let isCommutable = CommutableRR in {<br class="">
> def NAME#8rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi8 , opnode>;<br class="">
> - def NAME#16rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi16, opnode>;<br class="">
> - def NAME#32rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi32, opnode>;<br class="">
> - def NAME#64rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi64, opnode>;<br class="">
> + let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {<br class="">
> + def NAME#16rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi16, opnode>;<br class="">
> + def NAME#32rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi32, opnode>;<br class="">
> + def NAME#64rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi64, opnode>;<br class="">
> + } // isConvertibleToThreeAddress<br class="">
> } // isCommutable<br class="">
><br class="">
> def NAME#8rr_REV : BinOpRR_RFF_Rev<BaseOpc2, mnemonic, Xi8>;<br class="">
> @@ -1098,6 +1101,8 @@ multiclass ArithBinOp_RFF<bits<8> BaseOp<br class="">
> def NAME#32rm : BinOpRM_RFF<BaseOpc2, mnemonic, Xi32, opnode>;<br class="">
> def NAME#64rm : BinOpRM_RFF<BaseOpc2, mnemonic, Xi64, opnode>;<br class="">
><br class="">
> + def NAME#8ri : BinOpRI_RFF<0x80, mnemonic, Xi8 , opnode, RegMRM>;<br class="">
> +<br class="">
> let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {<br class="">
> // NOTE: These are order specific, we want the ri8 forms to be listed<br class="">
> // first so that they are slightly preferred to the ri forms.<br class="">
> @@ -1105,7 +1110,6 @@ multiclass ArithBinOp_RFF<bits<8> BaseOp<br class="">
> def NAME#32ri8 : BinOpRI8_RFF<0x82, mnemonic, Xi32, opnode, RegMRM>;<br class="">
> def NAME#64ri8 : BinOpRI8_RFF<0x82, mnemonic, Xi64, opnode, RegMRM>;<br class="">
><br class="">
> - def NAME#8ri : BinOpRI_RFF<0x80, mnemonic, Xi8 , opnode, RegMRM>;<br class="">
> def NAME#16ri : BinOpRI_RFF<0x80, mnemonic, Xi16, opnode, RegMRM>;<br class="">
> def NAME#32ri : BinOpRI_RFF<0x80, mnemonic, Xi32, opnode, RegMRM>;<br class="">
> def NAME#64ri32: BinOpRI_RFF<0x80, mnemonic, Xi64, opnode, RegMRM>;<br class="">
> @@ -1158,12 +1162,13 @@ multiclass ArithBinOp_F<bits<8> BaseOpc,<br class="">
> SDNode opnode,<br class="">
> bit CommutableRR, bit ConvertibleToThreeAddress> {<br class="">
> let Defs = [EFLAGS] in {<br class="">
> - let isCommutable = CommutableRR,<br class="">
> - isConvertibleToThreeAddress = ConvertibleToThreeAddress in {<br class="">
> + let isCommutable = CommutableRR in {<br class="">
> def NAME#8rr : BinOpRR_F<BaseOpc, mnemonic, Xi8 , opnode>;<br class="">
> - def NAME#16rr : BinOpRR_F<BaseOpc, mnemonic, Xi16, opnode>;<br class="">
> - def NAME#32rr : BinOpRR_F<BaseOpc, mnemonic, Xi32, opnode>;<br class="">
> - def NAME#64rr : BinOpRR_F<BaseOpc, mnemonic, Xi64, opnode>;<br class="">
> + let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {<br class="">
> + def NAME#16rr : BinOpRR_F<BaseOpc, mnemonic, Xi16, opnode>;<br class="">
> + def NAME#32rr : BinOpRR_F<BaseOpc, mnemonic, Xi32, opnode>;<br class="">
> + def NAME#64rr : BinOpRR_F<BaseOpc, mnemonic, Xi64, opnode>;<br class="">
> + }<br class="">
> } // isCommutable<br class="">
><br class="">
> def NAME#8rr_REV : BinOpRR_F_Rev<BaseOpc2, mnemonic, Xi8>;<br class="">
> @@ -1176,6 +1181,8 @@ multiclass ArithBinOp_F<bits<8> BaseOpc,<br class="">
> def NAME#32rm : BinOpRM_F<BaseOpc2, mnemonic, Xi32, opnode>;<br class="">
> def NAME#64rm : BinOpRM_F<BaseOpc2, mnemonic, Xi64, opnode>;<br class="">
><br class="">
> + def NAME#8ri : BinOpRI_F<0x80, mnemonic, Xi8 , opnode, RegMRM>;<br class="">
> +<br class="">
> let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {<br class="">
> // NOTE: These are order specific, we want the ri8 forms to be listed<br class="">
> // first so that they are slightly preferred to the ri forms.<br class="">
> @@ -1183,7 +1190,6 @@ multiclass ArithBinOp_F<bits<8> BaseOpc,<br class="">
> def NAME#32ri8 : BinOpRI8_F<0x82, mnemonic, Xi32, opnode, RegMRM>;<br class="">
> def NAME#64ri8 : BinOpRI8_F<0x82, mnemonic, Xi64, opnode, RegMRM>;<br class="">
><br class="">
> - def NAME#8ri : BinOpRI_F<0x80, mnemonic, Xi8 , opnode, RegMRM>;<br class="">
> def NAME#16ri : BinOpRI_F<0x80, mnemonic, Xi16, opnode, RegMRM>;<br class="">
> def NAME#32ri : BinOpRI_F<0x80, mnemonic, Xi32, opnode, RegMRM>;<br class="">
> def NAME#64ri32: BinOpRI_F<0x80, mnemonic, Xi64, opnode, RegMRM>;<br class="">
><br class="">
><br class="">
> _______________________________________________<br class="">
> llvm-commits mailing list<br class="">
> <a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br class="">
<br class="">
</blockquote></div><br class=""><br clear="all" class=""><div class=""><br class=""></div>-- <br class=""><div class="gmail_signature">~Craig</div>
</div>
</div></blockquote></div><br class=""></body></html>