Skip to content

Conversation

@9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Jan 13, 2015

https://issues.dlang.org/show_bug.cgi?id=13976

Example:

void main()
{
    int[10] a;
    size_t n;
    auto s = a[n%3 .. n%3 + 3];
    assert(s.length == 3);
}

With 2.066:

c:\d\test.d:1 void main()
00402010: c8380000                enter 0x38, 0x0
00402014: 53                      push ebx
00402015: 57                      push edi
c:\d\test.d:3     int[10] a;
00402016: b90a000000              mov ecx, 0xa
0040201b: 31c0                    xor eax, eax
0040201d: 8d7dc8                  lea edi, [ebp-0x38]
00402020: f3ab                    rep stosd
c:\d\test.d:4     size_t n;
00402022: 8945f0                  mov [ebp-0x10], eax
c:\d\test.d:5     auto s = a[n%3 .. n%3 + 3];
00402025: b103                    mov cl, 0x3
00402027: 31d2                    xor edx, edx
00402029: f7f1                    div ecx
0040202b: 8d5a03                  lea ebx, [edx+0x3]
0040202e: 83fb0a                  cmp ebx, 0xa
00402031: 7704                    ja 0x402037   D main c:\d\test.d:5
00402033: 39d3                    cmp ebx, edx
00402035: 730a                    jae 0x402041  D main c:\d\test.d:5
00402037: b805000000              mov eax, 0x5
0040203c: e823000000              call 0x402064 test.__array c:\d\test.d:7
00402041: 2bda                    sub ebx, edx
00402043: 8d5495c8                lea edx, [ebp+edx*4-0x38]
00402047: 895df8                  mov [ebp-0x8], ebx
0040204a: 8955fc                  mov [ebp-0x4], edx
c:\d\test.d:6     assert(s.length == 3);
0040204d: 394df8                  cmp [ebp-0x8], ecx
00402050: 740a                    jz 0x40205c   D main c:\d\test.d:6
00402052: b806000000              mov eax, 0x6
00402057: e824000000              call 0x402080 test.__assert c:\d\test.d:7
0040205c: 31c0                    xor eax, eax
c:\d\test.d:7 }
0040205e: 5f                      pop edi
0040205f: 5b                      pop ebx
00402060: c9                      leave
00402061: c3                      ret

With this PR:

c:\d\test.d:1 void main()
00402010: 55                      push ebp
00402011: 8bec                    mov ebp, esp
00402013: 83ec38                  sub esp, 0x38
00402016: 53                      push ebx
00402017: 57                      push edi
c:\d\test.d:3     int[10] a;
00402018: b90a000000              mov ecx, 0xa
0040201d: 31c0                    xor eax, eax
0040201f: 8d7dc8                  lea edi, [ebp-0x38]
00402022: f3ab                    rep stosd
c:\d\test.d:4     size_t n;
00402024: 8945f0                  mov [ebp-0x10], eax
c:\d\test.d:5     auto s = a[n%3 .. n%3 + 3];
00402027: b103                    mov cl, 0x3
00402029: 31d2                    xor edx, edx
0040202b: f7f1                    div ecx
0040202d: 8d5a03                  lea ebx, [edx+0x3]
00402030: 2bda                    sub ebx, edx
00402032: 8d5495c8                lea edx, [ebp+edx*4-0x38]
00402036: 895df8                  mov [ebp-0x8], ebx
00402039: 8955fc                  mov [ebp-0x4], edx
c:\d\test.d:6     assert(s.length == 3);
0040203c: 394df8                  cmp [ebp-0x8], ecx
0040203f: 740a                    jz 0x40204b   D main c:\d\test.d:6
00402041: b806000000              mov eax, 0x6
00402046: e809000000              call 0x402054 test.__assert c:\d\test.d:7
0040204b: 31c0                    xor eax, eax
c:\d\test.d:7 }
0040204d: 5f                      pop edi
0040204e: 5b                      pop ebx
0040204f: c9                      leave
00402050: c3                      ret

The boundary check and __array call are completely removed.

@9rnsr
Copy link
Contributor Author

9rnsr commented Jan 13, 2015

Ping: @WalterBright and @yebblies .

src/expression.h Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add comment indicating what these two fields mean, i.e. lowerLessThan means less than what?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, renamed two fields, and added explanation comments.

WalterBright added a commit that referenced this pull request Jan 14, 2015
Issue 13976 - Value range propagation to disable some slice bound tests
@WalterBright WalterBright merged commit 68260d6 into dlang:master Jan 14, 2015
@andralex
Copy link
Member

Thanks Kenji!!

@9rnsr 9rnsr deleted the fix13976 branch January 15, 2015 01:09
@nordlow
Copy link
Contributor

nordlow commented Jan 15, 2015

Does this include avoiding range checking in expressions such as

x[0 .. $/2, $/2 .. $]

which is a reoccurring pattern in D-style binary divide-and-conquer algorithms.

It would be super-cool if it worked for

rational variants with slice indexes in the form

$*p/q

where it can be statically proven that p <= q.

@don-clugston-sociomantic
Copy link
Contributor

@nordlow : all existing range checking only involves constants. There is no mechanism for ranges relative to variables, for example array lengths. Of course that would be awesome but it would be a massive new feature.

@nordlow
Copy link
Contributor

nordlow commented Jan 27, 2015

I take it that means lots of changes in lots of files in DMD, right?

@don-clugston-sociomantic
Copy link
Contributor

Well, it's a new algorithm, really. At the moment a value range is just two integers, lo and hi. Once you allow variables, it becomes an equation... So I dunno how extensive the changes would be.

@nordlow
Copy link
Contributor

nordlow commented Jan 27, 2015

Is the usage of IntRange defined in dmd/src/intrange.h a good place to start digging?

@don-clugston-sociomantic
Copy link
Contributor

Yeah. Though if you just wanted to do the $ case you mentioned, in SliceExp::optimize(), after each optimize(WANTvalue) you could replace each getIntRange call with a check for TOKmul, if it is TOKdivl(VarExp(__dollar), xxx) and xxx > 1 it is in range. You'd need to check all the cases though, it will get a bit messy.

@nordlow
Copy link
Contributor

nordlow commented Jan 27, 2015

Ok. Great. I guess I can set IndexExp::indexIsInBounds and SliceExp::upperIsInBounds when this can be proved, right?

Is this sufficient in order to trigger avoidance of range-checking further in the pipeline?

However, I'm missing SliceExp::lowerIsInBounds. Why isn't this defined?

@yebblies
Copy link
Contributor

However, I'm missing SliceExp::lowerIsInBounds. Why isn't this defined?

The variable you're looking for is lowerIsLessThanUpper.

@nordlow
Copy link
Contributor

nordlow commented Jan 27, 2015

How can low < high guarantee low >= 0?

@yebblies
Copy link
Contributor

How can low < high guarantee low >= 0?

Array indices are unsigned.

@nordlow
Copy link
Contributor

nordlow commented Jan 27, 2015

Ahh, I get it. Thanks.

@nordlow
Copy link
Contributor

nordlow commented Jan 28, 2015

Continued discussion at #4351

ibuclaw added a commit to ibuclaw/dmd that referenced this pull request Jan 11, 2018
ibuclaw added a commit to ibuclaw/dmd that referenced this pull request Jan 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants