<div dir="ltr">I considered moving them, but then load folding can't be done through patterns. With this change the only load folding that happens is because of the peephole optimizer, but that doesn't handle a lot of cases. So I guess it depends on how much we care about good load folding.</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Oct 3, 2013 at 11:45 AM, Yunzhong Gao <span dir="ltr"><<a href="mailto:Yunzhong_Gao@playstation.sony.com" target="_blank">Yunzhong_Gao@playstation.sony.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Craig,<br>
Do you remember why the BMI instruction selection was done by custom lowering instead of TableGen patterns? I tried to replace them with the following patterns, and there seems to be no visible changes. The tests in test/CodeGen/X86/bmi.ll are still passing just fine. Do you think this change is helpful?<br>
- Gao.<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D1822" target="_blank">http://llvm-reviews.chandlerc.com/D1822</a><br>
<br>
Files:<br>
lib/Target/X86/X86InstrInfo.td<br>
lib/Target/X86/X86ISelLowering.cpp<br>
lib/Target/X86/X86ISelLowering.h<br>
<br>
Index: lib/Target/X86/X86InstrInfo.td<br>
===================================================================<br>
--- lib/Target/X86/X86InstrInfo.td<br>
+++ lib/Target/X86/X86InstrInfo.td<br>
@@ -249,9 +249,6 @@<br>
def X86and_flag : SDNode<"X86ISD::AND", SDTBinaryArithWithFlags,<br>
[SDNPCommutative]>;<br>
<br>
-def X86blsi : SDNode<"X86ISD::BLSI", SDTIntUnaryOp>;<br>
-def X86blsmsk : SDNode<"X86ISD::BLSMSK", SDTIntUnaryOp>;<br>
-def X86blsr : SDNode<"X86ISD::BLSR", SDTIntUnaryOp>;<br>
def X86bzhi : SDNode<"X86ISD::BZHI", SDTIntShiftOp>;<br>
def X86bextr : SDNode<"X86ISD::BEXTR", SDTIntBinOp>;<br>
<br>
@@ -1809,30 +1806,22 @@<br>
}<br>
<br>
multiclass bmi_bls<string mnemonic, Format RegMRM, Format MemMRM,<br>
- RegisterClass RC, X86MemOperand x86memop, SDNode OpNode,<br>
- PatFrag ld_frag> {<br>
+ RegisterClass RC, X86MemOperand x86memop> {<br>
def rr : I<0xF3, RegMRM, (outs RC:$dst), (ins RC:$src),<br>
!strconcat(mnemonic, "\t{$src, $dst|$dst, $src}"),<br>
- [(set RC:$dst, (OpNode RC:$src)), (implicit EFLAGS)]>, T8, VEX_4V;<br>
+ []>, T8, VEX_4V;<br>
def rm : I<0xF3, MemMRM, (outs RC:$dst), (ins x86memop:$src),<br>
!strconcat(mnemonic, "\t{$src, $dst|$dst, $src}"),<br>
- [(set RC:$dst, (OpNode (ld_frag addr:$src))), (implicit EFLAGS)]>,<br>
- T8, VEX_4V;<br>
+ []>, T8, VEX_4V;<br>
}<br>
<br>
let Predicates = [HasBMI], Defs = [EFLAGS] in {<br>
- defm BLSR32 : bmi_bls<"blsr{l}", MRM1r, MRM1m, GR32, i32mem,<br>
- X86blsr, loadi32>;<br>
- defm BLSR64 : bmi_bls<"blsr{q}", MRM1r, MRM1m, GR64, i64mem,<br>
- X86blsr, loadi64>, VEX_W;<br>
- defm BLSMSK32 : bmi_bls<"blsmsk{l}", MRM2r, MRM2m, GR32, i32mem,<br>
- X86blsmsk, loadi32>;<br>
- defm BLSMSK64 : bmi_bls<"blsmsk{q}", MRM2r, MRM2m, GR64, i64mem,<br>
- X86blsmsk, loadi64>, VEX_W;<br>
- defm BLSI32 : bmi_bls<"blsi{l}", MRM3r, MRM3m, GR32, i32mem,<br>
- X86blsi, loadi32>;<br>
- defm BLSI64 : bmi_bls<"blsi{q}", MRM3r, MRM3m, GR64, i64mem,<br>
- X86blsi, loadi64>, VEX_W;<br>
+ defm BLSR32 : bmi_bls<"blsr{l}", MRM1r, MRM1m, GR32, i32mem>;<br>
+ defm BLSR64 : bmi_bls<"blsr{q}", MRM1r, MRM1m, GR64, i64mem>, VEX_W;<br>
+ defm BLSMSK32 : bmi_bls<"blsmsk{l}", MRM2r, MRM2m, GR32, i32mem>;<br>
+ defm BLSMSK64 : bmi_bls<"blsmsk{q}", MRM2r, MRM2m, GR64, i64mem>, VEX_W;<br>
+ defm BLSI32 : bmi_bls<"blsi{l}", MRM3r, MRM3m, GR32, i32mem>;<br>
+ defm BLSI64 : bmi_bls<"blsi{q}", MRM3r, MRM3m, GR64, i64mem>, VEX_W;<br>
}<br>
<br>
multiclass bmi_bextr_bzhi<bits<8> opc, string mnemonic, RegisterClass RC,<br>
@@ -1875,17 +1864,6 @@<br>
(BZHI64rm addr:$src1,<br>
(INSERT_SUBREG (i64 (IMPLICIT_DEF)), GR8:$src2, sub_8bit))>;<br>
<br>
-let Predicates = [HasBMI] in {<br>
- def : Pat<(X86bextr GR32:$src1, GR32:$src2),<br>
- (BEXTR32rr GR32:$src1, GR32:$src2)>;<br>
- def : Pat<(X86bextr (loadi32 addr:$src1), GR32:$src2),<br>
- (BEXTR32rm addr:$src1, GR32:$src2)>;<br>
- def : Pat<(X86bextr GR64:$src1, GR64:$src2),<br>
- (BEXTR64rr GR64:$src1, GR64:$src2)>;<br>
- def : Pat<(X86bextr (loadi64 addr:$src1), GR64:$src2),<br>
- (BEXTR64rm addr:$src1, GR64:$src2)>;<br>
-} // HasBMI<br>
-<br>
multiclass bmi_pdep_pext<string mnemonic, RegisterClass RC,<br>
X86MemOperand x86memop, Intrinsic Int,<br>
PatFrag ld_frag> {<br>
@@ -1910,6 +1888,36 @@<br>
}<br>
<br>
//===----------------------------------------------------------------------===//<br>
+// Pattern fragments to auto generate BMI instructions.<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+let Predicates = [HasBMI] in {<br>
+ def : Pat<(X86bextr GR32:$src1, GR32:$src2),<br>
+ (BEXTR32rr GR32:$src1, GR32:$src2)>;<br>
+ def : Pat<(X86bextr (loadi32 addr:$src1), GR32:$src2),<br>
+ (BEXTR32rm addr:$src1, GR32:$src2)>;<br>
+ def : Pat<(X86bextr GR64:$src1, GR64:$src2),<br>
+ (BEXTR64rr GR64:$src1, GR64:$src2)>;<br>
+ def : Pat<(X86bextr (loadi64 addr:$src1), GR64:$src2),<br>
+ (BEXTR64rm addr:$src1, GR64:$src2)>;<br>
+<br>
+ def : Pat<(and GR32:$src, (sub 0, GR32:$src)),<br>
+ (BLSI32rr GR32:$src)>;<br>
+ def : Pat<(and GR64:$src, (sub 0, GR64:$src)),<br>
+ (BLSI64rr GR64:$src)>;<br>
+<br>
+ def : Pat<(xor GR32:$src, (add GR32:$src, -1)),<br>
+ (BLSMSK32rr GR32:$src)>;<br>
+ def : Pat<(xor GR64:$src, (add GR64:$src, -1)),<br>
+ (BLSMSK64rr GR64:$src)>;<br>
+<br>
+ def : Pat<(and GR32:$src, (add GR32:$src, -1)),<br>
+ (BLSR32rr GR32:$src)>;<br>
+ def : Pat<(and GR64:$src, (add GR64:$src, -1)),<br>
+ (BLSR64rr GR64:$src)>;<br>
+} // HasBMI<br>
+<br>
+//===----------------------------------------------------------------------===//<br>
// TBM Instructions<br>
//<br>
let Predicates = [HasTBM], Defs = [EFLAGS] in {<br>
Index: lib/Target/X86/X86ISelLowering.cpp<br>
===================================================================<br>
--- lib/Target/X86/X86ISelLowering.cpp<br>
+++ lib/Target/X86/X86ISelLowering.cpp<br>
@@ -13710,9 +13710,6 @@<br>
case X86ISD::OR: return "X86ISD::OR";<br>
case X86ISD::XOR: return "X86ISD::XOR";<br>
case X86ISD::AND: return "X86ISD::AND";<br>
- case X86ISD::BLSI: return "X86ISD::BLSI";<br>
- case X86ISD::BLSMSK: return "X86ISD::BLSMSK";<br>
- case X86ISD::BLSR: return "X86ISD::BLSR";<br>
case X86ISD::BZHI: return "X86ISD::BZHI";<br>
case X86ISD::BEXTR: return "X86ISD::BEXTR";<br>
case X86ISD::MUL_IMM: return "X86ISD::MUL_IMM";<br>
@@ -17558,28 +17555,6 @@<br>
SDValue N1 = N->getOperand(1);<br>
SDLoc DL(N);<br>
<br>
- if (Subtarget->hasBMI()) {<br>
- // Check LHS for neg<br>
- if (N0.getOpcode() == ISD::SUB && N0.getOperand(1) == N1 &&<br>
- isZero(N0.getOperand(0)))<br>
- return DAG.getNode(X86ISD::BLSI, DL, VT, N1);<br>
-<br>
- // Check RHS for neg<br>
- if (N1.getOpcode() == ISD::SUB && N1.getOperand(1) == N0 &&<br>
- isZero(N1.getOperand(0)))<br>
- return DAG.getNode(X86ISD::BLSI, DL, VT, N0);<br>
-<br>
- // Check LHS for X-1<br>
- if (N0.getOpcode() == ISD::ADD && N0.getOperand(0) == N1 &&<br>
- isAllOnes(N0.getOperand(1)))<br>
- return DAG.getNode(X86ISD::BLSR, DL, VT, N1);<br>
-<br>
- // Check RHS for X-1<br>
- if (N1.getOpcode() == ISD::ADD && N1.getOperand(0) == N0 &&<br>
- isAllOnes(N1.getOperand(1)))<br>
- return DAG.getNode(X86ISD::BLSR, DL, VT, N0);<br>
- }<br>
-<br>
if (Subtarget->hasBMI2()) {<br>
// Check for (and (add (shl 1, Y), -1), X)<br>
if (N0.getOpcode() == ISD::ADD && isAllOnes(N0.getOperand(1))) {<br>
@@ -17853,28 +17828,6 @@<br>
return RV;<br>
}<br>
<br>
- // Try forming BMI if it is available.<br>
- if (!Subtarget->hasBMI())<br>
- return SDValue();<br>
-<br>
- if (VT != MVT::i32 && VT != MVT::i64)<br>
- return SDValue();<br>
-<br>
- assert(Subtarget->hasBMI() && "Creating BLSMSK requires BMI instructions");<br>
-<br>
- // Create BLSMSK instructions by finding X ^ (X-1)<br>
- SDValue N0 = N->getOperand(0);<br>
- SDValue N1 = N->getOperand(1);<br>
- SDLoc DL(N);<br>
-<br>
- if (N0.getOpcode() == ISD::ADD && N0.getOperand(0) == N1 &&<br>
- isAllOnes(N0.getOperand(1)))<br>
- return DAG.getNode(X86ISD::BLSMSK, DL, VT, N1);<br>
-<br>
- if (N1.getOpcode() == ISD::ADD && N1.getOperand(0) == N0 &&<br>
- isAllOnes(N1.getOperand(1)))<br>
- return DAG.getNode(X86ISD::BLSMSK, DL, VT, N0);<br>
-<br>
return SDValue();<br>
}<br>
<br>
Index: lib/Target/X86/X86ISelLowering.h<br>
===================================================================<br>
--- lib/Target/X86/X86ISelLowering.h<br>
+++ lib/Target/X86/X86ISelLowering.h<br>
@@ -292,9 +292,6 @@<br>
ADD, SUB, ADC, SBB, SMUL,<br>
INC, DEC, OR, XOR, AND,<br>
<br>
- BLSI, // BLSI - Extract lowest set isolated bit<br>
- BLSMSK, // BLSMSK - Get mask up to lowest set bit<br>
- BLSR, // BLSR - Reset lowest set bit<br>
BZHI, // BZHI - Zero high bits<br>
BEXTR, // BEXTR - Bit field extract<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br>~Craig
</div>