[llvm] r224940 - [X86] Fix some cases where some 8-bit instructions were marked as being convertible to three address instructions, but aren't really.
Quentin Colombet
qcolombet at apple.com
Mon Jan 5 13:43:59 PST 2015
> On Jan 5, 2015, at 12:27 PM, Craig Topper <craig.topper at gmail.com> wrote:
>
> 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.
I see, thanks for the clarification.
> Perhaps we should have an assertion in convertToThreeAddress in the default case of the switch to catch more of these mistagged instructions.
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.
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.
What do you think?
Thanks,
Q.
>
> On Mon, Jan 5, 2015 at 12:18 PM, Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
> Hi Craig,
>
> Would it be possible to have a test case for this?
>
> Thanks,
> -Quentin
>
> > On Dec 29, 2014, at 8:25 AM, Craig Topper <craig.topper at gmail.com <mailto:craig.topper at gmail.com>> wrote:
> >
> > Author: ctopper
> > Date: Mon Dec 29 10:25:26 2014
> > New Revision: 224940
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=224940&view=rev <http://llvm.org/viewvc/llvm-project?rev=224940&view=rev>
> > Log:
> > [X86] Fix some cases where some 8-bit instructions were marked as being convertible to three address instructions, but aren't really.
> >
> > Modified:
> > llvm/trunk/lib/Target/X86/X86InstrArithmetic.td
> >
> > Modified: llvm/trunk/lib/Target/X86/X86InstrArithmetic.td
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrArithmetic.td?rev=224940&r1=224939&r2=224940&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrArithmetic.td?rev=224940&r1=224939&r2=224940&view=diff>
> > ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86InstrArithmetic.td (original)
> > +++ llvm/trunk/lib/Target/X86/X86InstrArithmetic.td Mon Dec 29 10:25:26 2014
> > @@ -999,12 +999,13 @@ multiclass ArithBinOp_RF<bits<8> BaseOpc
> > bit CommutableRR, bit ConvertibleToThreeAddress> {
> > let Defs = [EFLAGS] in {
> > let Constraints = "$src1 = $dst" in {
> > - let isCommutable = CommutableRR,
> > - isConvertibleToThreeAddress = ConvertibleToThreeAddress in {
> > + let isCommutable = CommutableRR in {
> > def NAME#8rr : BinOpRR_RF<BaseOpc, mnemonic, Xi8 , opnodeflag>;
> > - def NAME#16rr : BinOpRR_RF<BaseOpc, mnemonic, Xi16, opnodeflag>;
> > - def NAME#32rr : BinOpRR_RF<BaseOpc, mnemonic, Xi32, opnodeflag>;
> > - def NAME#64rr : BinOpRR_RF<BaseOpc, mnemonic, Xi64, opnodeflag>;
> > + let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {
> > + def NAME#16rr : BinOpRR_RF<BaseOpc, mnemonic, Xi16, opnodeflag>;
> > + def NAME#32rr : BinOpRR_RF<BaseOpc, mnemonic, Xi32, opnodeflag>;
> > + def NAME#64rr : BinOpRR_RF<BaseOpc, mnemonic, Xi64, opnodeflag>;
> > + } // isConvertibleToThreeAddress
> > } // isCommutable
> >
> > def NAME#8rr_REV : BinOpRR_Rev<BaseOpc2, mnemonic, Xi8>;
> > @@ -1017,6 +1018,8 @@ multiclass ArithBinOp_RF<bits<8> BaseOpc
> > def NAME#32rm : BinOpRM_RF<BaseOpc2, mnemonic, Xi32, opnodeflag>;
> > def NAME#64rm : BinOpRM_RF<BaseOpc2, mnemonic, Xi64, opnodeflag>;
> >
> > + def NAME#8ri : BinOpRI_RF<0x80, mnemonic, Xi8 , opnodeflag, RegMRM>;
> > +
> > let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {
> > // NOTE: These are order specific, we want the ri8 forms to be listed
> > // first so that they are slightly preferred to the ri forms.
> > @@ -1024,7 +1027,6 @@ multiclass ArithBinOp_RF<bits<8> BaseOpc
> > def NAME#32ri8 : BinOpRI8_RF<0x82, mnemonic, Xi32, opnodeflag, RegMRM>;
> > def NAME#64ri8 : BinOpRI8_RF<0x82, mnemonic, Xi64, opnodeflag, RegMRM>;
> >
> > - def NAME#8ri : BinOpRI_RF<0x80, mnemonic, Xi8 , opnodeflag, RegMRM>;
> > def NAME#16ri : BinOpRI_RF<0x80, mnemonic, Xi16, opnodeflag, RegMRM>;
> > def NAME#32ri : BinOpRI_RF<0x80, mnemonic, Xi32, opnodeflag, RegMRM>;
> > def NAME#64ri32: BinOpRI_RF<0x80, mnemonic, Xi64, opnodeflag, RegMRM>;
> > @@ -1080,12 +1082,13 @@ multiclass ArithBinOp_RFF<bits<8> BaseOp
> > bit ConvertibleToThreeAddress> {
> > let Uses = [EFLAGS], Defs = [EFLAGS] in {
> > let Constraints = "$src1 = $dst" in {
> > - let isCommutable = CommutableRR,
> > - isConvertibleToThreeAddress = ConvertibleToThreeAddress in {
> > + let isCommutable = CommutableRR in {
> > def NAME#8rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi8 , opnode>;
> > - def NAME#16rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi16, opnode>;
> > - def NAME#32rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi32, opnode>;
> > - def NAME#64rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi64, opnode>;
> > + let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {
> > + def NAME#16rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi16, opnode>;
> > + def NAME#32rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi32, opnode>;
> > + def NAME#64rr : BinOpRR_RFF<BaseOpc, mnemonic, Xi64, opnode>;
> > + } // isConvertibleToThreeAddress
> > } // isCommutable
> >
> > def NAME#8rr_REV : BinOpRR_RFF_Rev<BaseOpc2, mnemonic, Xi8>;
> > @@ -1098,6 +1101,8 @@ multiclass ArithBinOp_RFF<bits<8> BaseOp
> > def NAME#32rm : BinOpRM_RFF<BaseOpc2, mnemonic, Xi32, opnode>;
> > def NAME#64rm : BinOpRM_RFF<BaseOpc2, mnemonic, Xi64, opnode>;
> >
> > + def NAME#8ri : BinOpRI_RFF<0x80, mnemonic, Xi8 , opnode, RegMRM>;
> > +
> > let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {
> > // NOTE: These are order specific, we want the ri8 forms to be listed
> > // first so that they are slightly preferred to the ri forms.
> > @@ -1105,7 +1110,6 @@ multiclass ArithBinOp_RFF<bits<8> BaseOp
> > def NAME#32ri8 : BinOpRI8_RFF<0x82, mnemonic, Xi32, opnode, RegMRM>;
> > def NAME#64ri8 : BinOpRI8_RFF<0x82, mnemonic, Xi64, opnode, RegMRM>;
> >
> > - def NAME#8ri : BinOpRI_RFF<0x80, mnemonic, Xi8 , opnode, RegMRM>;
> > def NAME#16ri : BinOpRI_RFF<0x80, mnemonic, Xi16, opnode, RegMRM>;
> > def NAME#32ri : BinOpRI_RFF<0x80, mnemonic, Xi32, opnode, RegMRM>;
> > def NAME#64ri32: BinOpRI_RFF<0x80, mnemonic, Xi64, opnode, RegMRM>;
> > @@ -1158,12 +1162,13 @@ multiclass ArithBinOp_F<bits<8> BaseOpc,
> > SDNode opnode,
> > bit CommutableRR, bit ConvertibleToThreeAddress> {
> > let Defs = [EFLAGS] in {
> > - let isCommutable = CommutableRR,
> > - isConvertibleToThreeAddress = ConvertibleToThreeAddress in {
> > + let isCommutable = CommutableRR in {
> > def NAME#8rr : BinOpRR_F<BaseOpc, mnemonic, Xi8 , opnode>;
> > - def NAME#16rr : BinOpRR_F<BaseOpc, mnemonic, Xi16, opnode>;
> > - def NAME#32rr : BinOpRR_F<BaseOpc, mnemonic, Xi32, opnode>;
> > - def NAME#64rr : BinOpRR_F<BaseOpc, mnemonic, Xi64, opnode>;
> > + let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {
> > + def NAME#16rr : BinOpRR_F<BaseOpc, mnemonic, Xi16, opnode>;
> > + def NAME#32rr : BinOpRR_F<BaseOpc, mnemonic, Xi32, opnode>;
> > + def NAME#64rr : BinOpRR_F<BaseOpc, mnemonic, Xi64, opnode>;
> > + }
> > } // isCommutable
> >
> > def NAME#8rr_REV : BinOpRR_F_Rev<BaseOpc2, mnemonic, Xi8>;
> > @@ -1176,6 +1181,8 @@ multiclass ArithBinOp_F<bits<8> BaseOpc,
> > def NAME#32rm : BinOpRM_F<BaseOpc2, mnemonic, Xi32, opnode>;
> > def NAME#64rm : BinOpRM_F<BaseOpc2, mnemonic, Xi64, opnode>;
> >
> > + def NAME#8ri : BinOpRI_F<0x80, mnemonic, Xi8 , opnode, RegMRM>;
> > +
> > let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {
> > // NOTE: These are order specific, we want the ri8 forms to be listed
> > // first so that they are slightly preferred to the ri forms.
> > @@ -1183,7 +1190,6 @@ multiclass ArithBinOp_F<bits<8> BaseOpc,
> > def NAME#32ri8 : BinOpRI8_F<0x82, mnemonic, Xi32, opnode, RegMRM>;
> > def NAME#64ri8 : BinOpRI8_F<0x82, mnemonic, Xi64, opnode, RegMRM>;
> >
> > - def NAME#8ri : BinOpRI_F<0x80, mnemonic, Xi8 , opnode, RegMRM>;
> > def NAME#16ri : BinOpRI_F<0x80, mnemonic, Xi16, opnode, RegMRM>;
> > def NAME#32ri : BinOpRI_F<0x80, mnemonic, Xi32, opnode, RegMRM>;
> > def NAME#64ri32: BinOpRI_F<0x80, mnemonic, Xi64, opnode, RegMRM>;
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>
>
>
>
> --
> ~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150105/043a8f27/attachment.html>
More information about the llvm-commits
mailing list