From 79752b6e63c03a18fd93817c6e9580d7445a7ce9 Mon Sep 17 00:00:00 2001 From: drh Date: Sat, 13 Aug 2016 10:02:17 +0000 Subject: [PATCH] Attempt to simplify the logic and generated code for vector comparisons. Basic comparison operators are working, but there are many indexing test failures still to be worked through. FossilOrigin-Name: dfc028cfbe7657d20727a2670ecadb1575eb8cbb --- manifest | 19 +++-- manifest.uuid | 2 +- src/expr.c | 109 ++++++++++++++------------- src/sqliteInt.h | 1 + src/vdbe.c | 195 ++++++++++++++++++++---------------------------- 5 files changed, 150 insertions(+), 176 deletions(-) diff --git a/manifest b/manifest index ca1fac3357..1a0b48fa4a 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Add\sVdbeCoverage()\smacros\son\snewly\sadded\sVDBE\sbranch\soperations. -D 2016-08-12T11:25:49.567 +C Attempt\sto\ssimplify\sthe\slogic\sand\sgenerated\scode\sfor\svector\scomparisons.\nBasic\scomparison\soperators\sare\sworking,\sbut\sthere\sare\smany\sindexing\stest\nfailures\sstill\sto\sbe\sworked\sthrough. +D 2016-08-13T10:02:17.801 F Makefile.in cfd8fb987cd7a6af046daa87daa146d5aad0e088 F Makefile.linux-gcc 7bc79876b875010e8c8f9502eb935ca92aa3c434 F Makefile.msc d66d0395c38571aab3804f8db0fa20707ae4609a @@ -338,7 +338,7 @@ F src/ctime.c e77f3dc297b4b65c96da78b4ae4272fdfae863d7 F src/date.c 95c9a8d00767e7221a8e9a31f4e913fc8029bf6b F src/dbstat.c 4f6f7f52b49beb9636ffbd517cfe44a402ba4ad0 F src/delete.c 4aba4214a377ce8ddde2d2e609777bcc8235200f -F src/expr.c d05cc249f8615bd4655f839ee57c24d11d005dde +F src/expr.c 375de68ad2daf3bd339f79074ced5a6db77e2f62 F src/fault.c 160a0c015b6c2629d3899ed2daf63d75754a32bb F src/fkey.c bc4145347595b7770f9a598cff1c848302cf5413 F src/func.c 29cc9acb170ec1387b9f63eb52cd85f8de96c771 @@ -389,7 +389,7 @@ F src/shell.c 79dda477be6c96eba6e952a934957ad36f87acc7 F src/sqlite.h.in 0f7580280d1b009b507d8beec1ff0f197ba0cc99 F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8 F src/sqlite3ext.h 8648034aa702469afb553231677306cc6492a1ae -F src/sqliteInt.h a1cf00afd6a5666a160e81c7a600418a3b59a8a6 +F src/sqliteInt.h 98d9ccfa30c0d4b1b886ed61f409dc6b307e9b0f F src/sqliteLimit.h c0373387c287c8d0932510b5547ecde31b5da247 F src/status.c a9e66593dfb28a9e746cba7153f84d49c1ddc4b1 F src/table.c 5226df15ab9179b9ed558d89575ea0ce37b03fc9 @@ -450,7 +450,7 @@ F src/update.c 4f05ea8cddfa367d045e03589756c02199e8f9bd F src/utf.c 699001c79f28e48e9bcdf8a463da029ea660540c F src/util.c 810ec3f22e2d1b62e66c30fe3621ebdedd23584d F src/vacuum.c 9dd2f5d276bc6094d8f1d85ecd41b30c1a002a43 -F src/vdbe.c 9f15129214a55044f918a983e1560fd87b0130a8 +F src/vdbe.c 9816bc4f89e4d58340f3cf3354fd062e2da11f8a F src/vdbe.h 67bc551f7faf04c33493892e4b378aada823ed10 F src/vdbeInt.h c59381049af5c7751a83456c39b80d1a6fde1f9d F src/vdbeapi.c c3f6715a99995c11748ecad91d25e93fd9fc390b @@ -1516,7 +1516,10 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 14009b32b955b42cfd5f0c2ce7d4b3ce19ce201e -R 57be00fa8f04c7e5888f286343bb13cf +P 381aa73141db8ec59adbcb09e71af660ee4ae5ce +R b082d1240e853643b05ab20adb3ffc1b +T *branch * vector-compare +T *sym-vector-compare * +T -sym-rowvalue * U drh -Z c05ca00f4c6d3d32267dd4171ca3bdf0 +Z 873e26b27a2fd221371ad3338313919b diff --git a/manifest.uuid b/manifest.uuid index e0b19abe4c..617135c700 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -381aa73141db8ec59adbcb09e71af660ee4ae5ce \ No newline at end of file +dfc028cfbe7657d20727a2670ecadb1575eb8cbb \ No newline at end of file diff --git a/src/expr.c b/src/expr.c index 3faeae79d2..54bf0dfaba 100644 --- a/src/expr.c +++ b/src/expr.c @@ -409,37 +409,52 @@ static int exprVectorRegister( /* ** Expression pExpr is a comparison between two vector values. Compute -** the result of the comparison and write it to register dest. +** the result of the comparison (1, 0, or NULL) and write that +** result into register dest. +** +** The caller must satisfy the following preconditions: +** +** if pExpr->op==TK_IS: op==TK_EQ and p5==SQLITE_NULLEQ +** if pExpr->op==TK_ISNOT: op==TK_NE and p5==SQLITE_NULLEQ +** otherwise: op==pExpr->op and p5==0 */ -static void codeVectorCompare(Parse *pParse, Expr *pExpr, int dest){ +static void codeVectorCompare( + Parse *pParse, /* Code generator context */ + Expr *pExpr, /* The comparison operation */ + int dest, /* Write results into this register */ + u8 op, /* Comparison operator */ + u8 p5 /* SQLITE_NULLEQ or zero */ +){ Vdbe *v = pParse->pVdbe; Expr *pLeft = pExpr->pLeft; Expr *pRight = pExpr->pRight; int nLeft = sqlite3ExprVectorSize(pLeft); int nRight = sqlite3ExprVectorSize(pRight); - int addr = sqlite3VdbeMakeLabel(v); /* Check that both sides of the comparison are vectors, and that ** both are the same length. */ if( nLeft!=nRight ){ sqlite3ErrorMsg(pParse, "invalid use of row value"); }else{ - int p5 = (pExpr->op==TK_IS || pExpr->op==TK_ISNOT) ? SQLITE_NULLEQ : 0; int i; int regLeft = 0; int regRight = 0; - int regTmp = 0; + u8 opx = op; + int addrDone = sqlite3VdbeMakeLabel(v); assert( pExpr->op==TK_EQ || pExpr->op==TK_NE || pExpr->op==TK_IS || pExpr->op==TK_ISNOT || pExpr->op==TK_LT || pExpr->op==TK_GT || pExpr->op==TK_LE || pExpr->op==TK_GE ); + assert( pExpr->op==op || (pExpr->op==TK_IS && op==TK_EQ) + || (pExpr->op==TK_ISNOT && op==TK_NE) ); + assert( p5==0 || pExpr->op!=op ); + assert( p5==SQLITE_NULLEQ || pExpr->op==op ); - if( pExpr->op==TK_EQ || pExpr->op==TK_NE ){ - regTmp = sqlite3GetTempReg(pParse); - sqlite3VdbeAddOp2(v, OP_Integer, (pExpr->op==TK_EQ), dest); - } + p5 |= SQLITE_STOREP2; + if( opx==TK_LE ) opx = TK_LT; + if( opx==TK_GE ) opx = TK_GT; regLeft = exprCodeSubselect(pParse, pLeft); regRight = exprCodeSubselect(pParse, pRight); @@ -448,55 +463,43 @@ static void codeVectorCompare(Parse *pParse, Expr *pExpr, int dest){ int regFree1 = 0, regFree2 = 0; Expr *pL, *pR; int r1, r2; - if( i ) sqlite3ExprCachePush(pParse); + if( i>0 ) sqlite3ExprCachePush(pParse); r1 = exprVectorRegister(pParse, pLeft, i, regLeft, &pL, ®Free1); r2 = exprVectorRegister(pParse, pRight, i, regRight, &pR, ®Free2); - - switch( pExpr->op ){ - case TK_IS: - codeCompare( - pParse, pL, pR, OP_Eq, r1, r2, dest, SQLITE_STOREP2|SQLITE_NULLEQ - ); - sqlite3VdbeAddOp3(v, OP_IfNot, dest, addr, 1); - VdbeCoverage(v); - break; - - case TK_ISNOT: - codeCompare( - pParse, pL, pR, OP_Ne, r1, r2, dest, SQLITE_STOREP2|SQLITE_NULLEQ - ); - sqlite3VdbeAddOp3(v, OP_If, dest, addr, 1); - VdbeCoverage(v); - break; - - case TK_EQ: - case TK_NE: - codeCompare(pParse, pL, pR, OP_Cmp, r1, r2, regTmp,SQLITE_STOREP2|p5); - sqlite3VdbeAddOp4Int( - v, OP_CmpTest, regTmp, addr, dest, pExpr->op==TK_NE - ); - VdbeCoverage(v); - break; - - case TK_LT: - case TK_LE: - case TK_GT: - case TK_GE: - codeCompare(pParse, pL, pR, OP_Cmp, r1, r2, dest, SQLITE_STOREP2|p5); - sqlite3VdbeAddOp4Int(v, OP_CmpTest, dest, addr, 0, pExpr->op); - VdbeCoverage(v); - break; - } - + codeCompare(pParse, pL, pR, opx, r1, r2, dest, p5); + testcase(op==OP_Lt); VdbeCoverageIf(v,op==OP_Lt); + testcase(op==OP_Le); VdbeCoverageIf(v,op==OP_Le); + testcase(op==OP_Gt); VdbeCoverageIf(v,op==OP_Gt); + testcase(op==OP_Ge); VdbeCoverageIf(v,op==OP_Ge); + testcase(op==OP_Eq); VdbeCoverageIf(v,op==OP_Eq); + testcase(op==OP_Ne); VdbeCoverageIf(v,op==OP_Ne); sqlite3ReleaseTempReg(pParse, regFree1); sqlite3ReleaseTempReg(pParse, regFree2); - if( i ) sqlite3ExprCachePop(pParse); + if( i>0 ) sqlite3ExprCachePop(pParse); + if( i==nLeft-1 ){ + break; + } + if( opx==TK_EQ ){ + sqlite3VdbeAddOp2(v, OP_IfNot, dest, addrDone); VdbeCoverage(v); + p5 |= SQLITE_KEEPNULL; + }else if( opx==TK_NE ){ + sqlite3VdbeAddOp2(v, OP_If, dest, addrDone); VdbeCoverage(v); + p5 |= SQLITE_KEEPNULL; + }else if( opx==op ){ + assert( op==TK_LT || op==TK_GT ); + sqlite3VdbeAddOp3(v, OP_If, dest, addrDone, 1); + VdbeCoverageIf(v, op==TK_LT); + VdbeCoverageIf(v, op==TK_GT); + }else{ + assert( op==TK_LE || op==TK_GE ); + sqlite3VdbeAddOp2(v, OP_ElseNotEq, 0, addrDone); + VdbeCoverageIf(v, op==TK_LE); + VdbeCoverageIf(v, op==TK_GE); + if( i==nLeft-2 ) opx = op; + } } - - sqlite3ReleaseTempReg(pParse, regTmp); + sqlite3VdbeResolveLabel(v, addrDone); } - - sqlite3VdbeResolveLabel(v, addr); } #if SQLITE_MAX_EXPR_DEPTH>0 @@ -3251,7 +3254,7 @@ int sqlite3ExprCodeTarget(Parse *pParse, Expr *pExpr, int target){ case TK_EQ: { Expr *pLeft = pExpr->pLeft; if( sqlite3ExprIsVector(pLeft) ){ - codeVectorCompare(pParse, pExpr, target); + codeVectorCompare(pParse, pExpr, target, op, p5); }else{ r1 = sqlite3ExprCodeTemp(pParse, pLeft, ®Free1); r2 = sqlite3ExprCodeTemp(pParse, pExpr->pRight, ®Free2); diff --git a/src/sqliteInt.h b/src/sqliteInt.h index a9061b2833..4c6554367f 100644 --- a/src/sqliteInt.h +++ b/src/sqliteInt.h @@ -1733,6 +1733,7 @@ struct CollSeq { ** operator is NULL. It is added to certain comparison operators to ** prove that the operands are always NOT NULL. */ +#define SQLITE_KEEPNULL 0x08 /* Used by vector == or <> */ #define SQLITE_JUMPIFNULL 0x10 /* jumps if either operand is NULL */ #define SQLITE_STOREP2 0x20 /* Store result in reg[P2] rather than jump */ #define SQLITE_NULLEQ 0x80 /* NULL=NULL */ diff --git a/src/vdbe.c b/src/vdbe.c index 054f1997f3..be5ca79ed5 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -585,6 +585,7 @@ int sqlite3VdbeExec( Mem *pOut = 0; /* Output operand */ int *aPermute = 0; /* Permutation of columns for OP_Compare */ i64 lastRowid = db->lastRowid; /* Saved value of the last insert ROWID */ + int cmpRes; /* Result of last comparison operation */ #ifdef VDBE_PROFILE u64 start; /* CPU clock count at start of opcode */ #endif @@ -1880,14 +1881,59 @@ case OP_Cast: { /* in1 */ } #endif /* SQLITE_OMIT_CAST */ +/* Opcode: Eq P1 P2 P3 P4 P5 +** Synopsis: if r[P1]==r[P3] goto P2 +** +** Compare the values in register P1 and P3. If reg(P3)==reg(P1) then +** jump to address P2. Or if the SQLITE_STOREP2 flag is set in P5, then +** store the result of comparison in register P2. +** +** The SQLITE_AFF_MASK portion of P5 must be an affinity character - +** SQLITE_AFF_TEXT, SQLITE_AFF_INTEGER, and so forth. An attempt is made +** to coerce both inputs according to this affinity before the +** comparison is made. If the SQLITE_AFF_MASK is 0x00, then numeric +** affinity is used. Note that the affinity conversions are stored +** back into the input registers P1 and P3. So this opcode can cause +** persistent changes to registers P1 and P3. +** +** Once any conversions have taken place, and neither value is NULL, +** the values are compared. If both values are blobs then memcmp() is +** used to determine the results of the comparison. If both values +** are text, then the appropriate collating function specified in +** P4 is used to do the comparison. If P4 is not specified then +** memcmp() is used to compare text string. If both values are +** numeric, then a numeric comparison is used. If the two values +** are of different types, then numbers are considered less than +** strings and strings are considered less than blobs. +** +** If SQLITE_NULLEQ is set in P5 then the result of comparison is always either +** true or false and is never NULL. If both operands are NULL then the result +** of comparison is true. If either operand is NULL then the result is false. +** If neither operand is NULL the result is the same as it would be if +** the SQLITE_NULLEQ flag were omitted from P5. +** +** If both SQLITE_STOREP2 and SQLITE_KEEPNULL flags are set then the +** content of r[P2] is only set to 1 (true) if it was not previously NULL. +*/ +/* Opcode: Ne P1 P2 P3 P4 P5 +** Synopsis: if r[P1]!=r[P3] goto P2 +** +** This works just like the Eq opcode except that the jump is taken if +** the operands in registers P1 and P3 are not equal. See the Eq opcode for +** additional information. +** +** If both SQLITE_STOREP2 and SQLITE_KEEPNULL flags are set then the +** content of r[P2] is only set to 0 (false) if it was not previously NULL. +*/ /* Opcode: Lt P1 P2 P3 P4 P5 ** Synopsis: if r[P1]flags */ u16 flags3; /* Copy of initial value of pIn3->flags */ - assert( pOp->opcode!=OP_Cmp || (pOp->p5 & SQLITE_STOREP2) ); pIn1 = &aMem[pOp->p1]; pIn3 = &aMem[pOp->p3]; flags1 = pIn1->flags; @@ -2002,15 +2004,16 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ && (flags3&MEM_Null)!=0 && (flags3&MEM_Cleared)==0 ){ - res = 0; /* Results are equal */ + cmpRes = 0; /* Results are equal */ }else{ - res = 1; /* Results are not equal */ + cmpRes = 1; /* Results are not equal */ } }else{ /* SQLITE_NULLEQ is clear and at least one operand is NULL, ** then the result is always NULL. ** The jump is taken if the SQLITE_JUMPIFNULL bit is set. */ + cmpRes = 1; if( pOp->p5 & SQLITE_STOREP2 ){ pOut = &aMem[pOp->p2]; memAboutToChange(p, pOut); @@ -2063,16 +2066,15 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ sqlite3VdbeMemExpandBlob(pIn3); flags3 &= ~MEM_Zero; } - res = sqlite3MemCompare(pIn3, pIn1, pOp->p4.pColl); + cmpRes = sqlite3MemCompare(pIn3, pIn1, pOp->p4.pColl); } switch( pOp->opcode ){ - case OP_Eq: res = res==0; break; - case OP_Ne: res = res!=0; break; - case OP_Lt: res = res<0; break; - case OP_Le: res = res<=0; break; - case OP_Gt: res = res>0; break; - case OP_Ge: res = res>=0; break; - default: assert( pOp->opcode==OP_Cmp ); break; + case OP_Eq: res = cmpRes==0; break; + case OP_Ne: res = cmpRes!=0; break; + case OP_Lt: res = cmpRes<0; break; + case OP_Le: res = cmpRes<=0; break; + case OP_Gt: res = cmpRes>0; break; + case OP_Ge: res = cmpRes>=0; break; } /* Undo any changes made by applyAffinity() to the input registers. */ @@ -2083,12 +2085,18 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ if( pOp->p5 & SQLITE_STOREP2 ){ pOut = &aMem[pOp->p2]; + if( (pOp->p5 & SQLITE_KEEPNULL)!=0 && (pOut->flags & MEM_Null)!=0 ){ + /* The KEEPNULL flag prevents OP_Eq from overwriting a NULL with 1 + ** and prevents OP_Ne from overwriting NULL with 0. */ + assert( pOp->opcode==OP_Ne || pOp->opcode==OP_Eq ); + assert( res==0 || res==1 ); + if( (pOp->opcode==OP_Eq)==res ) break; + } memAboutToChange(p, pOut); MemSetTypeFlag(pOut, MEM_Int); pOut->u.i = res; REGISTER_TRACE(pOp->p2, pOut); }else{ - assert( pOp->opcode!=OP_Cmp ); VdbeBranchTaken(res!=0, (pOp->p5 & SQLITE_NULLEQ)?2:3); if( res ){ goto jump_to_p2; @@ -2097,6 +2105,22 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ break; } +/* Opcode: ElseNotEq * P2 * * * +** +** This opcode must immediately follow an Lt or Gt comparison operator. +** If the operands in that previous comparison are not equal (possibly +** because one or the other is NULL) then jump to P2. If the two operands +** of the prior comparison are equal, fall through. +*/ +case OP_ElseNotEq: { /* same as TK_ESCAPE, jump */ + assert( pOp>aOp ); + assert( pOp[-1].opcode==OP_Lt || pOp[-1].opcode==OP_Gt ); + VdbeBranchTaken(cmpRes!=0, 2); + if( cmpRes!=0 ) goto jump_to_p2; + break; +} + + /* Opcode: Permutation * * * P4 * ** ** Set the permutation used by the OP_Compare operator to be the array @@ -3885,63 +3909,6 @@ seek_not_found: break; } -/* Opcode: CmpTest P1 P2 P3 P4 * -** -** P2 is a jump destination. Register P1 is guaranteed to contain either -** an integer value or a NULL. -** -** If P3 is non-zero, it identifies an output register. In this case, if -** P1 is NULL, P3 is also set to NULL. Or, if P1 is any integer value -** other than 0, P3 is set to the value of P4 and a jump to P2 is taken. -** -** If P3 is 0, the jump is taken if P1 contains any value other than 0 (i.e. -** NULL does cause a jump). Additionally, if P1 is not NULL, its value is -** modified to integer value 0 or 1 according to the value of the P4 integer -** operand: -** -** P4 modification -** -------------------------- -** OP_Lt (P1 = (P1 < 0)) -** OP_Le (P1 = (P1 <= 0)) -** OP_Gt (P1 = (P1 > 0)) -** OP_Ge (P1 = (P1 >= 0)) -*/ -case OP_CmpTest: { /* in1, jump */ - int bJump; - pIn1 = &aMem[pOp->p1]; - - if( pOp->p3 ){ - bJump = 0; - if( pIn1->flags & MEM_Null ){ - memAboutToChange(p, &aMem[pOp->p3]); - MemSetTypeFlag(&aMem[pOp->p3], MEM_Null); - }else if( pIn1->u.i!=0 ){ - memAboutToChange(p, &aMem[pOp->p3]); - MemSetTypeFlag(&aMem[pOp->p3], MEM_Int); - aMem[pOp->p3].u.i = pOp->p4.i; - bJump = 1; - } - }else{ - if( (pIn1->flags & MEM_Int) ){ - bJump = (pIn1->u.i!=0); - switch( pOp->p4.i ){ - case OP_Lt: pIn1->u.i = (pIn1->u.i < 0); break; - case OP_Le: pIn1->u.i = (pIn1->u.i <= 0); break; - case OP_Gt: pIn1->u.i = (pIn1->u.i > 0); break; - default: - assert( pOp->p4.i==OP_Ge ); - pIn1->u.i = (pIn1->u.i >= 0); - break; - } - }else{ - bJump = 1; - } - } - - if( bJump ) goto jump_to_p2; - break; -} - /* Opcode: Found P1 P2 P3 P4 * ** Synopsis: key=r[P3@P4] **