<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/go-git.git/test, branch dev.boringcrypto.go1.12</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>[release-branch.go1.12] cmd/link: increase the function call limit in stkcheck</title>
<updated>2019-08-09T21:08:07+00:00</updated>
<author>
<name>Ian Lance Taylor</name>
<email>iant@golang.org</email>
</author>
<published>2019-08-09T17:54:43+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=4e15604ada268612c2c81402e172627c7f22f483'/>
<id>4e15604ada268612c2c81402e172627c7f22f483</id>
<content type='text'>
There is real (albeit generated) code that exceeds the limit.

Updates #33555
Fixes #33557

Change-Id: I668e85825d3d2a471970e869abe63f3492213cc1
Reviewed-on: https://go-review.googlesource.com/c/go/+/189697
Run-TryBot: Ian Lance Taylor &lt;iant@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Cherry Zhang &lt;cherryyz@google.com&gt;
(cherry picked from commit 951143cf1425189965498cc32fc8dc132bf7c777)
Reviewed-on: https://go-review.googlesource.com/c/go/+/189717
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
There is real (albeit generated) code that exceeds the limit.

Updates #33555
Fixes #33557

Change-Id: I668e85825d3d2a471970e869abe63f3492213cc1
Reviewed-on: https://go-review.googlesource.com/c/go/+/189697
Run-TryBot: Ian Lance Taylor &lt;iant@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Cherry Zhang &lt;cherryyz@google.com&gt;
(cherry picked from commit 951143cf1425189965498cc32fc8dc132bf7c777)
Reviewed-on: https://go-review.googlesource.com/c/go/+/189717
</pre>
</div>
</content>
</entry>
<entry>
<title>[release-branch.go1.12] cmd/compile: add necessary operand to mergePoint in rewrite rules</title>
<updated>2019-07-08T17:49:50+00:00</updated>
<author>
<name>David Chase</name>
<email>drchase@google.com</email>
</author>
<published>2019-06-19T22:09:39+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=4dbe72de5fc7e32f829a97135b2d7a8f7984f33a'/>
<id>4dbe72de5fc7e32f829a97135b2d7a8f7984f33a</id>
<content type='text'>
A missing operand to mergePoint caused lower to place values
in the wrong blocks.

Includes test, belt+suspenders to do both ssa check and verify
the output (was is how the bug was originally observed).

The fixed bug here is very likely present in Go versions
1.9-1.12 on amd64 and s390x

Updates #32680.
Fixes #32712.

Change-Id: I63e702c4c40602cb795ef71b1691eb704d38ccc7
Reviewed-on: https://go-review.googlesource.com/c/go/+/183059
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;
(cherry picked from commit 769fda2d5110eef6146d7df3bf0219872c9b0da6)
Reviewed-on: https://go-review.googlesource.com/c/go/+/183241
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
A missing operand to mergePoint caused lower to place values
in the wrong blocks.

Includes test, belt+suspenders to do both ssa check and verify
the output (was is how the bug was originally observed).

The fixed bug here is very likely present in Go versions
1.9-1.12 on amd64 and s390x

Updates #32680.
Fixes #32712.

Change-Id: I63e702c4c40602cb795ef71b1691eb704d38ccc7
Reviewed-on: https://go-review.googlesource.com/c/go/+/183059
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;
(cherry picked from commit 769fda2d5110eef6146d7df3bf0219872c9b0da6)
Reviewed-on: https://go-review.googlesource.com/c/go/+/183241
</pre>
</div>
</content>
</entry>
<entry>
<title>[release-branch.go1.12] cmd/compile: fix range analysis of small signed integers</title>
<updated>2019-07-01T17:23:23+00:00</updated>
<author>
<name>Matthew Dempsky</name>
<email>mdempsky@google.com</email>
</author>
<published>2019-06-12T02:52:58+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=39b533ed6634079a5c4f6a3488f4b01cdab2d833'/>
<id>39b533ed6634079a5c4f6a3488f4b01cdab2d833</id>
<content type='text'>
For int8, int16, and int32, comparing their unsigned value to MaxInt64
to determine non-negativity doesn't make sense, because they have
negative values whose unsigned representation is smaller than that.
Fix is simply to compare with the appropriate upper bound based on the
value type's size.

Fixes #32583.

Change-Id: Ie7afad7a56af92bd890ba5ff33c86d1df06cfd9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/181797
Run-TryBot: Matthew Dempsky &lt;mdempsky@google.com&gt;
Reviewed-by: Josh Bleecher Snyder &lt;josharian@gmail.com&gt;
Reviewed-by: 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;
(cherry picked from commit f44404ebbfeff57f3e45ebf4b314a320bb89841f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/181978
Reviewed-by: Emmanuel Odeke &lt;emm.odeke@gmail.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
For int8, int16, and int32, comparing their unsigned value to MaxInt64
to determine non-negativity doesn't make sense, because they have
negative values whose unsigned representation is smaller than that.
Fix is simply to compare with the appropriate upper bound based on the
value type's size.

Fixes #32583.

Change-Id: Ie7afad7a56af92bd890ba5ff33c86d1df06cfd9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/181797
Run-TryBot: Matthew Dempsky &lt;mdempsky@google.com&gt;
Reviewed-by: Josh Bleecher Snyder &lt;josharian@gmail.com&gt;
Reviewed-by: 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;
(cherry picked from commit f44404ebbfeff57f3e45ebf4b314a320bb89841f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/181978
Reviewed-by: Emmanuel Odeke &lt;emm.odeke@gmail.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>[release-branch.go1.12] cmd/link: fix deferreturn detector</title>
<updated>2019-06-10T19:51:25+00:00</updated>
<author>
<name>Keith Randall</name>
<email>keithr@alum.mit.edu</email>
</author>
<published>2019-06-07T17:22:53+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=a6178d8ee32c3a53cd489373061d475b52cc289e'/>
<id>a6178d8ee32c3a53cd489373061d475b52cc289e</id>
<content type='text'>
The logic for detecting deferreturn calls is wrong.

We used to look for a relocation whose symbol is runtime.deferreturn
and has an offset of 0. But on some architectures, the relocation
offset is not zero. These include arm (the offset is 0xebfffffe) and
s390x (the offset is 6).

This ends up setting the deferreturn offset at 0, so we end up using
the entry point live map instead of the deferreturn live map in a
frame which defers and then segfaults.

Instead, use the IsDirectJump helper to find calls.

Fixes #32484

Change-Id: Iecb530a7cf6eabd7233be7d0731ffa78873f3a54
Reviewed-on: https://go-review.googlesource.com/c/go/+/181258
Run-TryBot: Keith Randall &lt;khr@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Cherry Zhang &lt;cherryyz@google.com&gt;
(cherry picked from commit 9eb403159da9debbb4881140995e62bec0c943f3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/181262
Reviewed-by: Emmanuel Odeke &lt;emm.odeke@gmail.com&gt;
Run-TryBot: Emmanuel Odeke &lt;emm.odeke@gmail.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The logic for detecting deferreturn calls is wrong.

We used to look for a relocation whose symbol is runtime.deferreturn
and has an offset of 0. But on some architectures, the relocation
offset is not zero. These include arm (the offset is 0xebfffffe) and
s390x (the offset is 6).

This ends up setting the deferreturn offset at 0, so we end up using
the entry point live map instead of the deferreturn live map in a
frame which defers and then segfaults.

Instead, use the IsDirectJump helper to find calls.

Fixes #32484

Change-Id: Iecb530a7cf6eabd7233be7d0731ffa78873f3a54
Reviewed-on: https://go-review.googlesource.com/c/go/+/181258
Run-TryBot: Keith Randall &lt;khr@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Cherry Zhang &lt;cherryyz@google.com&gt;
(cherry picked from commit 9eb403159da9debbb4881140995e62bec0c943f3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/181262
Reviewed-by: Emmanuel Odeke &lt;emm.odeke@gmail.com&gt;
Run-TryBot: Emmanuel Odeke &lt;emm.odeke@gmail.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>[release-branch.go1.12] cmd/compile: make sure to initialize static entries of slices</title>
<updated>2019-05-14T21:16:32+00:00</updated>
<author>
<name>Keith Randall</name>
<email>khr@google.com</email>
</author>
<published>2019-05-13T20:43:49+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=ecf7695c7dcfc04becde0beec0e1c5f583b42e9e'/>
<id>ecf7695c7dcfc04becde0beec0e1c5f583b42e9e</id>
<content type='text'>
If a slice's entries are sparse, we decide to initialize it dynamically
instead of statically. That's CL 151319.

But if we do initialize it dynamically, we still need to initialize
the static entries. Typically we do that, but the bug fixed here is
that we don't if the entry's value is itself an array or struct.

To fix, use initKindLocalCode to ensure that both static and
dynamic entries are initialized via code.

Fixes #32013

Change-Id: I1192ffdbfb5cd50445c1206c4a3d8253295201dd
Reviewed-on: https://go-review.googlesource.com/c/go/+/176904
Run-TryBot: Keith Randall &lt;khr@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Josh Bleecher Snyder &lt;josharian@gmail.com&gt;
(cherry picked from commit a9e107c85cf69d735ac81c29f4a354643e40b2b5)
Reviewed-on: https://go-review.googlesource.com/c/go/+/177040
Reviewed-by: Brad Fitzpatrick &lt;bradfitz@golang.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
If a slice's entries are sparse, we decide to initialize it dynamically
instead of statically. That's CL 151319.

But if we do initialize it dynamically, we still need to initialize
the static entries. Typically we do that, but the bug fixed here is
that we don't if the entry's value is itself an array or struct.

To fix, use initKindLocalCode to ensure that both static and
dynamic entries are initialized via code.

Fixes #32013

Change-Id: I1192ffdbfb5cd50445c1206c4a3d8253295201dd
Reviewed-on: https://go-review.googlesource.com/c/go/+/176904
Run-TryBot: Keith Randall &lt;khr@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Josh Bleecher Snyder &lt;josharian@gmail.com&gt;
(cherry picked from commit a9e107c85cf69d735ac81c29f4a354643e40b2b5)
Reviewed-on: https://go-review.googlesource.com/c/go/+/177040
Reviewed-by: Brad Fitzpatrick &lt;bradfitz@golang.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>[release-branch.go1.12] cmd/compile: use correct package name for stack object symbol</title>
<updated>2019-05-06T18:53:43+00:00</updated>
<author>
<name>Keith Randall</name>
<email>keithr@alum.mit.edu</email>
</author>
<published>2019-04-10T23:44:46+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=a17d091120c26c982ca0cde3ac590f49b08fd196'/>
<id>a17d091120c26c982ca0cde3ac590f49b08fd196</id>
<content type='text'>
Stack object generation code was always using the local package name
for its symbol. Normally that doesn't matter, as we usually only
compile functions in the local package. But for wrappers, the compiler
generates functions which live in other packages. When there are two
other packages with identical functions to wrap, the same name appears
twice, and the compiler goes boom.

Fixes #31396

Change-Id: I7026eebabe562cb159b8b6046cf656afd336ba25
Reviewed-on: https://go-review.googlesource.com/c/go/+/171464
Run-TryBot: Keith Randall &lt;khr@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Matthew Dempsky &lt;mdempsky@google.com&gt;
(cherry picked from commit 43001a0dc96a29f662f2782c5fb3ca998eadd623)
Reviewed-on: https://go-review.googlesource.com/c/go/+/173317
Reviewed-by: Brad Fitzpatrick &lt;bradfitz@golang.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Stack object generation code was always using the local package name
for its symbol. Normally that doesn't matter, as we usually only
compile functions in the local package. But for wrappers, the compiler
generates functions which live in other packages. When there are two
other packages with identical functions to wrap, the same name appears
twice, and the compiler goes boom.

Fixes #31396

Change-Id: I7026eebabe562cb159b8b6046cf656afd336ba25
Reviewed-on: https://go-review.googlesource.com/c/go/+/171464
Run-TryBot: Keith Randall &lt;khr@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Matthew Dempsky &lt;mdempsky@google.com&gt;
(cherry picked from commit 43001a0dc96a29f662f2782c5fb3ca998eadd623)
Reviewed-on: https://go-review.googlesource.com/c/go/+/173317
Reviewed-by: Brad Fitzpatrick &lt;bradfitz@golang.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>[release-branch.go1.12] cmd/compile: fix literal struct interface {} lost passing by value</title>
<updated>2019-04-02T19:43:35+00:00</updated>
<author>
<name>LE Manh Cuong</name>
<email>cuong.manhle.vn@gmail.com</email>
</author>
<published>2019-03-22T18:43:06+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=991583017c856a14431382794a2a2f98ca52e537'/>
<id>991583017c856a14431382794a2a2f98ca52e537</id>
<content type='text'>
CL 135377 introduces pass strings and slices to convT2{E,I} by value.
Before that CL, all types, except interface will be allocated temporary
address. The CL changes the logic that only constant and type which
needs address (determine by convFuncName) will be allocated.

It fails to cover the case where type is static composite literal.
Adding condition to check that case fixes the issue.

Also, static composite literal node implies constant type, so consttype
checking can be removed.

Fixes #31209

Change-Id: Ifc750a029fb4889c2d06e73e44bf85e6ef4ce881
Reviewed-on: https://go-review.googlesource.com/c/go/+/168858
Run-TryBot: Ian Lance Taylor &lt;iant@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Keith Randall &lt;khr@golang.org&gt;
(cherry picked from commit d47db6dc0ce72d1371c81a677b45d7bdba39ff46)
Reviewed-on: https://go-review.googlesource.com/c/go/+/170437
Run-TryBot: Andrew Bonventre &lt;andybons@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>
CL 135377 introduces pass strings and slices to convT2{E,I} by value.
Before that CL, all types, except interface will be allocated temporary
address. The CL changes the logic that only constant and type which
needs address (determine by convFuncName) will be allocated.

It fails to cover the case where type is static composite literal.
Adding condition to check that case fixes the issue.

Also, static composite literal node implies constant type, so consttype
checking can be removed.

Fixes #31209

Change-Id: Ifc750a029fb4889c2d06e73e44bf85e6ef4ce881
Reviewed-on: https://go-review.googlesource.com/c/go/+/168858
Run-TryBot: Ian Lance Taylor &lt;iant@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Keith Randall &lt;khr@golang.org&gt;
(cherry picked from commit d47db6dc0ce72d1371c81a677b45d7bdba39ff46)
Reviewed-on: https://go-review.googlesource.com/c/go/+/170437
Run-TryBot: Andrew Bonventre &lt;andybons@golang.org&gt;
Reviewed-by: Cherry Zhang &lt;cherryyz@google.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>[release-branch.go1.12] cmd/compile: copy volatile values before emitting write barrier call</title>
<updated>2019-04-02T05:05:37+00:00</updated>
<author>
<name>Cherry Zhang</name>
<email>cherryyz@google.com</email>
</author>
<published>2019-03-21T17:40:28+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=8acc2ea68ba126c6f91afef2545fd6022b039c51'/>
<id>8acc2ea68ba126c6f91afef2545fd6022b039c51</id>
<content type='text'>
It is possible that a "volatile" value (one that can be clobbered
by preparing args of a call) to be used in multiple write barrier
calls. We used to copy the volatile value right before each call.
But this doesn't work if the value is used the second time, after
the first call where it is already clobbered. Copy it before
emitting any call.

Updates #30977.
Fixes #30996.

Change-Id: Iedcc91ad848d5ded547bf37a8359c125d32e994c
Reviewed-on: https://go-review.googlesource.com/c/go/+/168677
Run-TryBot: Cherry Zhang &lt;cherryyz@google.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Keith Randall &lt;khr@golang.org&gt;
(cherry picked from commit f23c601bf9f66ef4a0d9d2dcd003b95e78fb28f8)
Reviewed-on: https://go-review.googlesource.com/c/go/+/168817
Run-TryBot: Brad Fitzpatrick &lt;bradfitz@golang.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
It is possible that a "volatile" value (one that can be clobbered
by preparing args of a call) to be used in multiple write barrier
calls. We used to copy the volatile value right before each call.
But this doesn't work if the value is used the second time, after
the first call where it is already clobbered. Copy it before
emitting any call.

Updates #30977.
Fixes #30996.

Change-Id: Iedcc91ad848d5ded547bf37a8359c125d32e994c
Reviewed-on: https://go-review.googlesource.com/c/go/+/168677
Run-TryBot: Cherry Zhang &lt;cherryyz@google.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Keith Randall &lt;khr@golang.org&gt;
(cherry picked from commit f23c601bf9f66ef4a0d9d2dcd003b95e78fb28f8)
Reviewed-on: https://go-review.googlesource.com/c/go/+/168817
Run-TryBot: Brad Fitzpatrick &lt;bradfitz@golang.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>[release-branch.go1.12] cmd/compile: make KeepAlive work on stack object</title>
<updated>2019-03-13T20:59:47+00:00</updated>
<author>
<name>Cherry Zhang</name>
<email>cherryyz@google.com</email>
</author>
<published>2019-02-28T01:43:29+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=6fc1242ea8f8bbacd88cb834ef61dbe9e84a8514'/>
<id>6fc1242ea8f8bbacd88cb834ef61dbe9e84a8514</id>
<content type='text'>
Currently, runtime.KeepAlive applied on a stack object doesn't
actually keeps the stack object alive, and the heap object
referenced from it could be collected. This is because the
address of the stack object is rematerializeable, and we just
ignored KeepAlive on rematerializeable values. This CL fixes it.

Updates #30476.
Fixes #30478.

Change-Id: Ic1f75ee54ed94ea79bd46a8ddcd9e81d01556d1d
Reviewed-on: https://go-review.googlesource.com/c/164537
Run-TryBot: Cherry Zhang &lt;cherryyz@google.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Keith Randall &lt;khr@golang.org&gt;
(cherry picked from commit 40df9cc6062492cd323f2251dd1583d200d1207e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/164627
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Currently, runtime.KeepAlive applied on a stack object doesn't
actually keeps the stack object alive, and the heap object
referenced from it could be collected. This is because the
address of the stack object is rematerializeable, and we just
ignored KeepAlive on rematerializeable values. This CL fixes it.

Updates #30476.
Fixes #30478.

Change-Id: Ic1f75ee54ed94ea79bd46a8ddcd9e81d01556d1d
Reviewed-on: https://go-review.googlesource.com/c/164537
Run-TryBot: Cherry Zhang &lt;cherryyz@google.com&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: Keith Randall &lt;khr@golang.org&gt;
(cherry picked from commit 40df9cc6062492cd323f2251dd1583d200d1207e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/164627
</pre>
</div>
</content>
</entry>
<entry>
<title>[release-branch.go1.12] cmd/compile: fix ordering for short-circuiting ops</title>
<updated>2019-03-13T19:54:41+00:00</updated>
<author>
<name>Keith Randall</name>
<email>khr@google.com</email>
</author>
<published>2019-03-06T01:42:48+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=f062f48c1f8862ff0ef7baccf563497339129743'/>
<id>f062f48c1f8862ff0ef7baccf563497339129743</id>
<content type='text'>
Make sure the side effects inside short-circuited operations (&amp;&amp; and ||)
happen correctly.

Before this CL, we attached the side effects to the node itself using
exprInPlace. That caused other side effects in sibling expressions
to get reordered with respect to the short circuit side effect.

Instead, rewrite a &amp;&amp; b like:

r := a
if r {
  r = b
}

That code we can keep correctly ordered with respect to other
side-effects extracted from part of a big expression.

exprInPlace seems generally unsafe. But this was the only case where
exprInPlace is called not at the top level of an expression, so I
don't think the other uses can actually trigger an issue (there can't
be a sibling expression). TODO: maybe those cases don't need "in
place", and we can retire that function generally.

This CL needed a small tweak to the SSA generation of OIF so that the
short circuit optimization still triggers. The short circuit optimization
looks for triangle but not diamonds, so don't bother allocating a block
if it will be empty.

Go 1 benchmarks are in the noise.

Fixes #30567

Change-Id: I19c04296bea63cbd6ad05f87a63b005029123610
Reviewed-on: https://go-review.googlesource.com/c/go/+/165617
Run-TryBot: Keith Randall &lt;khr@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: David Chase &lt;drchase@google.com&gt;
(cherry picked from commit 4a9064ef41ccc65454564536f40cf7d5a00db8ad)
Reviewed-on: https://go-review.googlesource.com/c/go/+/165858
Reviewed-by: Brad Fitzpatrick &lt;bradfitz@golang.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Make sure the side effects inside short-circuited operations (&amp;&amp; and ||)
happen correctly.

Before this CL, we attached the side effects to the node itself using
exprInPlace. That caused other side effects in sibling expressions
to get reordered with respect to the short circuit side effect.

Instead, rewrite a &amp;&amp; b like:

r := a
if r {
  r = b
}

That code we can keep correctly ordered with respect to other
side-effects extracted from part of a big expression.

exprInPlace seems generally unsafe. But this was the only case where
exprInPlace is called not at the top level of an expression, so I
don't think the other uses can actually trigger an issue (there can't
be a sibling expression). TODO: maybe those cases don't need "in
place", and we can retire that function generally.

This CL needed a small tweak to the SSA generation of OIF so that the
short circuit optimization still triggers. The short circuit optimization
looks for triangle but not diamonds, so don't bother allocating a block
if it will be empty.

Go 1 benchmarks are in the noise.

Fixes #30567

Change-Id: I19c04296bea63cbd6ad05f87a63b005029123610
Reviewed-on: https://go-review.googlesource.com/c/go/+/165617
Run-TryBot: Keith Randall &lt;khr@golang.org&gt;
TryBot-Result: Gobot Gobot &lt;gobot@golang.org&gt;
Reviewed-by: David Chase &lt;drchase@google.com&gt;
(cherry picked from commit 4a9064ef41ccc65454564536f40cf7d5a00db8ad)
Reviewed-on: https://go-review.googlesource.com/c/go/+/165858
Reviewed-by: Brad Fitzpatrick &lt;bradfitz@golang.org&gt;
</pre>
</div>
</content>
</entry>
</feed>
