<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/go-git.git/test/loopbce.go, branch dev.typeparams</title>
<subtitle>github.com: golang/go
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/'/>
<entry>
<title>cmd/compile: detect indvars that are bound by other indvars</title>
<updated>2019-09-26T18:47:12+00:00</updated>
<author>
<name>Giovanni Bajo</name>
<email>rasky@develer.com</email>
</author>
<published>2019-09-16T08:25:48+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=1658263bbfbf0c31f179df878049e6d4690501c8'/>
<id>1658263bbfbf0c31f179df878049e6d4690501c8</id>
<content type='text'>
prove wasn't able to detect induction variables that was bound
by another inducation variable. This happened because an indvar
is a Phi, and thus in case of a dependency, the loop bounding
condition looked as Phi &lt; Phi. This triggered an existing
codepath that checked whether the upper bound was a Phi to
detect loop conditions written in reversed order respect to the
idiomatic way (eg: for i:=0; len(n)&gt;i; i++).

To fix this, we call the indvar pattern matching on both operands
of the loop condition, so that the first operand that matches
will be treated as the indvar.

Updates #24660 (removes a boundcheck from Fannkuch)

Change-Id: Iade83d8deb54f14277ed3f2e37b190e1ed173d11
Reviewed-on: https://go-review.googlesource.com/c/go/+/195220
Reviewed-by: David Chase &lt;drchase@google.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
prove wasn't able to detect induction variables that was bound
by another inducation variable. This happened because an indvar
is a Phi, and thus in case of a dependency, the loop bounding
condition looked as Phi &lt; Phi. This triggered an existing
codepath that checked whether the upper bound was a Phi to
detect loop conditions written in reversed order respect to the
idiomatic way (eg: for i:=0; len(n)&gt;i; i++).

To fix this, we call the indvar pattern matching on both operands
of the loop condition, so that the first operand that matches
will be treated as the indvar.

Updates #24660 (removes a boundcheck from Fannkuch)

Change-Id: Iade83d8deb54f14277ed3f2e37b190e1ed173d11
Reviewed-on: https://go-review.googlesource.com/c/go/+/195220
Reviewed-by: David Chase &lt;drchase@google.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>cmd/compile: reverse order of slice bounds checks</title>
<updated>2019-03-09T00:52:45+00:00</updated>
<author>
<name>Keith Randall</name>
<email>keithr@alum.mit.edu</email>
</author>
<published>2019-03-08T23:01:32+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=83a33d3855e257b383b2a3a10dfd9748ad17cfb4'/>
<id>83a33d3855e257b383b2a3a10dfd9748ad17cfb4</id>
<content type='text'>
Turns out this makes the fix for 28797 unnecessary, because this order
ensures that the RHS of IsSliceInBounds ops are always nonnegative.

The real reason for this change is that it also makes dealing with
&lt;0 values easier for reporting values in bounds check panics (issue #30116).

Makes cmd/go negligibly smaller.

Update #28797

Change-Id: I1f25ba6d2b3b3d4a72df3105828aa0a4b629ce85
Reviewed-on: https://go-review.googlesource.com/c/go/+/166377
Run-TryBot: Keith Randall &lt;khr@golang.org&gt;
Reviewed-by: Brad Fitzpatrick &lt;bradfitz@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Turns out this makes the fix for 28797 unnecessary, because this order
ensures that the RHS of IsSliceInBounds ops are always nonnegative.

The real reason for this change is that it also makes dealing with
&lt;0 values easier for reporting values in bounds check panics (issue #30116).

Makes cmd/go negligibly smaller.

Update #28797

Change-Id: I1f25ba6d2b3b3d4a72df3105828aa0a4b629ce85
Reviewed-on: https://go-review.googlesource.com/c/go/+/166377
Run-TryBot: Keith Randall &lt;khr@golang.org&gt;
Reviewed-by: Brad Fitzpatrick &lt;bradfitz@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>cmd/compile: check for negative upper bound to IsSliceInBounds</title>
<updated>2018-12-07T23:04:58+00:00</updated>
<author>
<name>David Chase</name>
<email>drchase@google.com</email>
</author>
<published>2018-12-04T15:00:16+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=ea6259d5e9d57f247b7d877d4d04602b74ae5155'/>
<id>ea6259d5e9d57f247b7d877d4d04602b74ae5155</id>
<content type='text'>
IsSliceInBounds(x, y) asserts that y is not negative, but
there were cases where this is not true.  Change code
generation to ensure that this is true when it's not obviously
true.  Prove phase cleans a few of these out.

With this change the compiler text section is 0.06% larger,
that is, not very much.  Benchmarking still TBD, may need
to wait for access to a benchmarking box (next week).

Also corrected run.go to handle '?' in -update_errors output.

Fixes #28797.

Change-Id: Ia8af90bc50a91ae6e934ef973def8d3f398fac7b
Reviewed-on: https://go-review.googlesource.com/c/152477
Run-TryBot: David Chase &lt;drchase@google.com&gt;
Reviewed-by: Keith Randall &lt;khr@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
IsSliceInBounds(x, y) asserts that y is not negative, but
there were cases where this is not true.  Change code
generation to ensure that this is true when it's not obviously
true.  Prove phase cleans a few of these out.

With this change the compiler text section is 0.06% larger,
that is, not very much.  Benchmarking still TBD, may need
to wait for access to a benchmarking box (next week).

Also corrected run.go to handle '?' in -update_errors output.

Fixes #28797.

Change-Id: Ia8af90bc50a91ae6e934ef973def8d3f398fac7b
Reviewed-on: https://go-review.googlesource.com/c/152477
Run-TryBot: David Chase &lt;drchase@google.com&gt;
Reviewed-by: Keith Randall &lt;khr@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>cmd/compile: ensure that loop condition is detected correctly</title>
<updated>2018-07-09T18:23:39+00:00</updated>
<author>
<name>Keith Randall</name>
<email>khr@google.com</email>
</author>
<published>2018-07-02T22:21:35+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=58d287e5e863cd8d3c3525e1a04e424e748cf242'/>
<id>58d287e5e863cd8d3c3525e1a04e424e748cf242</id>
<content type='text'>
We need to make sure that the terminating comparison has the right
sense given the increment direction. If the increment is positive,
the terminating comparsion must be &lt; or &lt;=. If the increment is
negative, the terminating comparison must be &gt; or &gt;=.

Do a few cleanups,  like constant-folding entry==0, adding comments,
removing unused "exported" fields.

Fixes #26116

Change-Id: I14230ee8126054b750e2a1f2b18eb8f09873dbd5
Reviewed-on: https://go-review.googlesource.com/121940
Run-TryBot: Keith Randall &lt;khr@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Heschi Kreinick &lt;heschi@google.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
We need to make sure that the terminating comparison has the right
sense given the increment direction. If the increment is positive,
the terminating comparsion must be &lt; or &lt;=. If the increment is
negative, the terminating comparison must be &gt; or &gt;=.

Do a few cleanups,  like constant-folding entry==0, adding comments,
removing unused "exported" fields.

Fixes #26116

Change-Id: I14230ee8126054b750e2a1f2b18eb8f09873dbd5
Reviewed-on: https://go-review.googlesource.com/121940
Run-TryBot: Keith Randall &lt;khr@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Heschi Kreinick &lt;heschi@google.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>cmd/compile: teach prove to handle expressions like len(s)-delta</title>
<updated>2018-04-29T09:38:32+00:00</updated>
<author>
<name>Giovanni Bajo</name>
<email>rasky@develer.com</email>
</author>
<published>2018-04-15T14:52:49+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=e0d37a33ab6260f5acc68dbb9a02c3135d19bcbb'/>
<id>e0d37a33ab6260f5acc68dbb9a02c3135d19bcbb</id>
<content type='text'>
When a loop has bound len(s)-delta, findIndVar detected it and
returned len(s) as (conservative) upper bound. This little lie
allowed loopbce to drop bound checks.

It is obviously more generic to teach prove about relations like
x+d&lt;w for non-constant "w"; we already handled the case for
constant "w", so we just want to learn that if d&lt;0, then x+d&lt;w
proves that x&lt;w.

To be able to remove the code from findIndVar, we also need
to teach prove that len() and cap() are always non-negative.

This CL allows to prove 633 more checks in cmd+std. Most
of them are cases where the code was already testing before
accessing a slice but the compiler didn't know it. For instance,
take strings.HasSuffix:

    func HasSuffix(s, suffix string) bool {
        return len(s) &gt;= len(suffix) &amp;&amp; s[len(s)-len(suffix):] == suffix
    }

When suffix is a literal string, the compiler now understands
that the explicit check is enough to not emit a slice check.

I also found a loopbce test that was incorrectly
written to detect an overflow but had a off-by-one (on the
conservative side), so it unexpectly passed with this CL; I
changed it to really trigger the overflow as intended.

Change-Id: Ib5abade337db46b8811425afebad4719b6e46c4a
Reviewed-on: https://go-review.googlesource.com/105635
Run-TryBot: Giovanni Bajo &lt;rasky@develer.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: David Chase &lt;drchase@google.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When a loop has bound len(s)-delta, findIndVar detected it and
returned len(s) as (conservative) upper bound. This little lie
allowed loopbce to drop bound checks.

It is obviously more generic to teach prove about relations like
x+d&lt;w for non-constant "w"; we already handled the case for
constant "w", so we just want to learn that if d&lt;0, then x+d&lt;w
proves that x&lt;w.

To be able to remove the code from findIndVar, we also need
to teach prove that len() and cap() are always non-negative.

This CL allows to prove 633 more checks in cmd+std. Most
of them are cases where the code was already testing before
accessing a slice but the compiler didn't know it. For instance,
take strings.HasSuffix:

    func HasSuffix(s, suffix string) bool {
        return len(s) &gt;= len(suffix) &amp;&amp; s[len(s)-len(suffix):] == suffix
    }

When suffix is a literal string, the compiler now understands
that the explicit check is enough to not emit a slice check.

I also found a loopbce test that was incorrectly
written to detect an overflow but had a off-by-one (on the
conservative side), so it unexpectly passed with this CL; I
changed it to really trigger the overflow as intended.

Change-Id: Ib5abade337db46b8811425afebad4719b6e46c4a
Reviewed-on: https://go-review.googlesource.com/105635
Run-TryBot: Giovanni Bajo &lt;rasky@develer.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: David Chase &lt;drchase@google.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>cmd/compile: in prove, detect loops with negative increments</title>
<updated>2018-04-29T09:38:18+00:00</updated>
<author>
<name>Giovanni Bajo</name>
<email>rasky@develer.com</email>
</author>
<published>2018-04-15T14:03:30+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=6d379add0fefcc17ed7b763078526800a3c1d705'/>
<id>6d379add0fefcc17ed7b763078526800a3c1d705</id>
<content type='text'>
To be effective, this also requires being able to relax constraints
on min/max bound inclusiveness; they are now exposed through a flags,
and prove has been updated to handle it correctly.

Change-Id: I3490e54461b7b9de8bc4ae40d3b5e2fa2d9f0556
Reviewed-on: https://go-review.googlesource.com/104041
Run-TryBot: Giovanni Bajo &lt;rasky@develer.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: David Chase &lt;drchase@google.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
To be effective, this also requires being able to relax constraints
on min/max bound inclusiveness; they are now exposed through a flags,
and prove has been updated to handle it correctly.

Change-Id: I3490e54461b7b9de8bc4ae40d3b5e2fa2d9f0556
Reviewed-on: https://go-review.googlesource.com/104041
Run-TryBot: Giovanni Bajo &lt;rasky@develer.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: David Chase &lt;drchase@google.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>cmd/compile: improve testing of induction variables</title>
<updated>2018-04-29T09:38:09+00:00</updated>
<author>
<name>Giovanni Bajo</name>
<email>rasky@develer.com</email>
</author>
<published>2018-04-02T01:17:18+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=980fdb8dd5fe0151a9b7e84ec6b8c20a11727521'/>
<id>980fdb8dd5fe0151a9b7e84ec6b8c20a11727521</id>
<content type='text'>
Test both minimum and maximum bound, and prepare
formatting for more advanced tests (inclusive / esclusive bounds).

Change-Id: Ibe432916d9c938343bc07943798bc9709ad71845
Reviewed-on: https://go-review.googlesource.com/104040
Run-TryBot: Giovanni Bajo &lt;rasky@develer.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Brad Fitzpatrick &lt;bradfitz@golang.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Test both minimum and maximum bound, and prepare
formatting for more advanced tests (inclusive / esclusive bounds).

Change-Id: Ibe432916d9c938343bc07943798bc9709ad71845
Reviewed-on: https://go-review.googlesource.com/104040
Run-TryBot: Giovanni Bajo &lt;rasky@develer.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Brad Fitzpatrick &lt;bradfitz@golang.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>cmd/compile: implement loop BCE in prove</title>
<updated>2018-04-29T09:37:35+00:00</updated>
<author>
<name>Giovanni Bajo</name>
<email>rasky@develer.com</email>
</author>
<published>2018-04-01T23:57:49+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=7ec25d0acfed3f40fe634be518f0857704e5b642'/>
<id>7ec25d0acfed3f40fe634be518f0857704e5b642</id>
<content type='text'>
Reuse findIndVar to discover induction variables, and then
register the facts we know about them into the facts table
when entering the loop block.

Moreover, handle "x+delta &gt; w" while updating the facts table,
to be able to prove accesses to slices with constant offsets
such as slice[i-10].

Change-Id: I2a63d050ed58258136d54712ac7015b25c893d71
Reviewed-on: https://go-review.googlesource.com/104038
Run-TryBot: Giovanni Bajo &lt;rasky@develer.com&gt;
Reviewed-by: David Chase &lt;drchase@google.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Reuse findIndVar to discover induction variables, and then
register the facts we know about them into the facts table
when entering the loop block.

Moreover, handle "x+delta &gt; w" while updating the facts table,
to be able to prove accesses to slices with constant offsets
such as slice[i-10].

Change-Id: I2a63d050ed58258136d54712ac7015b25c893d71
Reviewed-on: https://go-review.googlesource.com/104038
Run-TryBot: Giovanni Bajo &lt;rasky@develer.com&gt;
Reviewed-by: David Chase &lt;drchase@google.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>cmd/compile: make loop guard+rotate conditional on GOEXPERIMENT</title>
<updated>2017-06-21T22:07:33+00:00</updated>
<author>
<name>David Chase</name>
<email>drchase@google.com</email>
</author>
<published>2017-06-21T21:19:24+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=0b6fbaae6ec4218fc35d8a6f4415ae8f88198301'/>
<id>0b6fbaae6ec4218fc35d8a6f4415ae8f88198301</id>
<content type='text'>
Loops of the form "for i,e := range" needed to have their
condition rotated to the "bottom" for the preemptible loops
GOEXPERIMENT, but this caused a performance regression
because it degraded bounds check removal.  For now, make
the loop rotation/guarding conditional on the experiment.

Fixes #20711.
Updates #10958.

Change-Id: Icfba14cb3b13a910c349df8f84838cf4d9d20cf6
Reviewed-on: https://go-review.googlesource.com/46410
Run-TryBot: David Chase &lt;drchase@google.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Keith Randall &lt;khr@golang.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Loops of the form "for i,e := range" needed to have their
condition rotated to the "bottom" for the preemptible loops
GOEXPERIMENT, but this caused a performance regression
because it degraded bounds check removal.  For now, make
the loop rotation/guarding conditional on the experiment.

Fixes #20711.
Updates #10958.

Change-Id: Icfba14cb3b13a910c349df8f84838cf4d9d20cf6
Reviewed-on: https://go-review.googlesource.com/46410
Run-TryBot: David Chase &lt;drchase@google.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Keith Randall &lt;khr@golang.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>cmd/compile: check loop rescheduling with stack bound, not counter</title>
<updated>2017-03-08T18:52:12+00:00</updated>
<author>
<name>David Chase</name>
<email>drchase@google.com</email>
</author>
<published>2017-02-02T16:53:41+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=d71f36b5aa1eadc6cd86ada2c0d5dd621bd9fd82'/>
<id>d71f36b5aa1eadc6cd86ada2c0d5dd621bd9fd82</id>
<content type='text'>
After benchmarking with a compiler modified to have better
spill location, it became clear that this method of checking
was actually faster on (at least) two different architectures
(ppc64 and amd64) and it also provides more timely interruption
of loops.

This change adds a modified FOR loop node "FORUNTIL" that
checks after executing the loop body instead of before (i.e.,
always at least once).  This ensures that a pointer past the
end of a slice or array is not made visible to the garbage
collector.

Without the rescheduling checks inserted, the restructured
loop from this  change apparently provides a 1% geomean
improvement on PPC64 running the go1 benchmarks; the
improvement on AMD64 is only 0.12%.

Inserting the rescheduling check exposed some peculiar bug
with the ssa test code for s390x; this was updated based on
initial code actually generated for GOARCH=s390x to use
appropriate OpArg, OpAddr, and OpVarDef.

NaCl is disabled in testing.

Change-Id: Ieafaa9a61d2a583ad00968110ef3e7a441abca50
Reviewed-on: https://go-review.googlesource.com/36206
Run-TryBot: David Chase &lt;drchase@google.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Cherry Zhang &lt;cherryyz@google.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
After benchmarking with a compiler modified to have better
spill location, it became clear that this method of checking
was actually faster on (at least) two different architectures
(ppc64 and amd64) and it also provides more timely interruption
of loops.

This change adds a modified FOR loop node "FORUNTIL" that
checks after executing the loop body instead of before (i.e.,
always at least once).  This ensures that a pointer past the
end of a slice or array is not made visible to the garbage
collector.

Without the rescheduling checks inserted, the restructured
loop from this  change apparently provides a 1% geomean
improvement on PPC64 running the go1 benchmarks; the
improvement on AMD64 is only 0.12%.

Inserting the rescheduling check exposed some peculiar bug
with the ssa test code for s390x; this was updated based on
initial code actually generated for GOARCH=s390x to use
appropriate OpArg, OpAddr, and OpVarDef.

NaCl is disabled in testing.

Change-Id: Ieafaa9a61d2a583ad00968110ef3e7a441abca50
Reviewed-on: https://go-review.googlesource.com/36206
Run-TryBot: David Chase &lt;drchase@google.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Cherry Zhang &lt;cherryyz@google.com&gt;
</pre>
</div>
</content>
</entry>
</feed>
