Skip to content

Commit

Permalink
cmd/compile: ensure constant folding of pointer arithmetic remains a …
Browse files Browse the repository at this point in the history
…pointer

For c + nil, we want the result to still be of pointer type.

Fixes ppc64le build failure with CL 468455, in issue33724.go.

The problem in that test is that it requires a nil check to be
scheduled before the corresponding load. This normally happens fine
because we prioritize nil checks. If we have nilcheck(p) and load(p),
once p is scheduled the nil check will always go before the load.

The issue we saw in 33724 is that when p is a nil pointer, we ended up
with two different p's, an int64(0) as the argument to the nil check
and an (*Outer)(0) as the argument to the load. Those two zeroes don't
get CSEd, so if the (*Outer)(0) happens to get scheduled first, the
load can end up before the nilcheck.

Fix this by always having constant arithmetic preserve the pointerness
of the value, so that both zeroes are of type *Outer and get CSEd.

Update golang#58482
Update golang#33724

Change-Id: Ib9b8c0446f1690b574e0f3c0afb9934efbaf3513
Reviewed-on: https://go-review.googlesource.com/c/go/+/468615
Reviewed-by: Keith Randall <[email protected]>
Run-TryBot: Keith Randall <[email protected]>
Reviewed-by: David Chase <[email protected]>
TryBot-Bypass: Keith Randall <[email protected]>
  • Loading branch information
randall77 committed Feb 17, 2023
1 parent 031401a commit 21f4340
Show file tree
Hide file tree
Showing 22 changed files with 65 additions and 34 deletions.
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/_gen/386.rules
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@
(NE (TESTB (SETNEF cmp) (SETNEF cmp)) yes no) => (NEF cmp yes no)

// fold constants into instructions
(ADDL x (MOVLconst [c])) => (ADDLconst [c] x)
(ADDL x (MOVLconst <t> [c])) && !t.IsPtr() => (ADDLconst [c] x)
(ADDLcarry x (MOVLconst [c])) => (ADDLconstcarry [c] x)
(ADCL x (MOVLconst [c]) f) => (ADCLconst [c] x f)

Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/_gen/AMD64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@
// (SETEQF x) => (ANDQ (SETEQ <typ.Int8> x) (SETORD <typ.Int8> x))

// fold constants into instructions
(ADDQ x (MOVQconst [c])) && is32Bit(c) => (ADDQconst [int32(c)] x)
(ADDQ x (MOVQconst <t> [c])) && is32Bit(c) && !t.IsPtr() => (ADDQconst [int32(c)] x)
(ADDQ x (MOVLconst [c])) => (ADDQconst [c] x)
(ADDL x (MOVLconst [c])) => (ADDLconst [c] x)

Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/_gen/ARM.rules
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@
(MOVHloadidx ptr idx (MOVHstoreidx ptr2 idx x _)) && isSamePtr(ptr, ptr2) => (MOVHreg x)

// fold constant into arithmetic ops
(ADD x (MOVWconst [c])) => (ADDconst [c] x)
(ADD x (MOVWconst <t> [c])) && !t.IsPtr() => (ADDconst [c] x)
(SUB (MOVWconst [c]) x) => (RSBconst [c] x)
(SUB x (MOVWconst [c])) => (SUBconst [c] x)
(RSB (MOVWconst [c]) x) => (SUBconst [c] x)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/_gen/ARM64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@
(MOVDnop (MOVDconst [c])) => (MOVDconst [c])

// fold constant into arithmetic ops
(ADD x (MOVDconst [c])) => (ADDconst [c] x)
(ADD x (MOVDconst <t> [c])) && !t.IsPtr() => (ADDconst [c] x)
(SUB x (MOVDconst [c])) => (SUBconst [c] x)
(AND x (MOVDconst [c])) => (ANDconst [c] x)
(OR x (MOVDconst [c])) => (ORconst [c] x)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/_gen/LOONG64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@
(MOVVreg x) && x.Uses == 1 => (MOVVnop x)

// fold constant into arithmetic ops
(ADDV x (MOVVconst [c])) && is32Bit(c) => (ADDVconst [c] x)
(ADDV x (MOVVconst <t> [c])) && is32Bit(c) && !t.IsPtr() => (ADDVconst [c] x)
(SUBV x (MOVVconst [c])) && is32Bit(c) => (SUBVconst [c] x)
(AND x (MOVVconst [c])) && is32Bit(c) => (ANDconst [c] x)
(OR x (MOVVconst [c])) && is32Bit(c) => (ORconst [c] x)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/_gen/MIPS.rules
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@
(MOVWnop (MOVWconst [c])) => (MOVWconst [c])

// fold constant into arithmetic ops
(ADD x (MOVWconst [c])) => (ADDconst [c] x)
(ADD x (MOVWconst <t> [c])) && !t.IsPtr() => (ADDconst [c] x)
(SUB x (MOVWconst [c])) => (SUBconst [c] x)
(AND x (MOVWconst [c])) => (ANDconst [c] x)
(OR x (MOVWconst [c])) => (ORconst [c] x)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/_gen/MIPS64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@
(MOVVnop (MOVVconst [c])) => (MOVVconst [c])

// fold constant into arithmetic ops
(ADDV x (MOVVconst [c])) && is32Bit(c) => (ADDVconst [c] x)
(ADDV x (MOVVconst <t> [c])) && is32Bit(c) && !t.IsPtr() => (ADDVconst [c] x)
(SUBV x (MOVVconst [c])) && is32Bit(c) => (SUBVconst [c] x)
(AND x (MOVVconst [c])) && is32Bit(c) => (ANDconst [c] x)
(OR x (MOVVconst [c])) && is32Bit(c) => (ORconst [c] x)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/_gen/PPC64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@

// Arithmetic constant ops

(ADD x (MOVDconst [c])) && is32Bit(c) => (ADDconst [c] x)
(ADD x (MOVDconst <t> [c])) && is32Bit(c) && !t.IsPtr() => (ADDconst [c] x)
(ADDconst [c] (ADDconst [d] x)) && is32Bit(c+d) => (ADDconst [c+d] x)
(ADDconst [0] x) => x
(SUB x (MOVDconst [c])) && is32Bit(-c) => (ADDconst [-c] x)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/_gen/RISCV64.rules
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@
(MOVDnop (MOVDconst [c])) => (MOVDconst [c])

// Fold constant into immediate instructions where possible.
(ADD (MOVDconst [val]) x) && is32Bit(val) => (ADDI [val] x)
(ADD (MOVDconst <t> [val]) x) && is32Bit(val) && !t.IsPtr() => (ADDI [val] x)
(AND (MOVDconst [val]) x) && is32Bit(val) => (ANDI [val] x)
(OR (MOVDconst [val]) x) && is32Bit(val) => (ORI [val] x)
(XOR (MOVDconst [val]) x) && is32Bit(val) => (XORI [val] x)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/_gen/S390X.rules
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@
(BRC {c} (CMPWUconst x [y]) yes no) && y == int32( int8(y)) && (c == s390x.Equal || c == s390x.LessOrGreater) => (CIJ {c} x [ int8(y)] yes no)

// Fold constants into instructions.
(ADD x (MOVDconst [c])) && is32Bit(c) => (ADDconst [int32(c)] x)
(ADD x (MOVDconst <t> [c])) && is32Bit(c) && !t.IsPtr() => (ADDconst [int32(c)] x)
(ADDW x (MOVDconst [c])) => (ADDWconst [int32(c)] x)

(SUB x (MOVDconst [c])) && is32Bit(c) => (SUBconst x [int32(c)])
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/_gen/Wasm.rules
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@
(I64LeU (I64Const [1]) x) => (I64Eqz (I64Eqz x))
(I64Ne x (I64Const [0])) => (I64Eqz (I64Eqz x))

(I64Add x (I64Const [y])) => (I64AddConst [y] x)
(I64Add x (I64Const <t> [y])) && !t.IsPtr() => (I64AddConst [y] x)
(I64AddConst [0] x) => x
(I64Eqz (I64Eqz (I64Eqz x))) => (I64Eqz x)

Expand Down
7 changes: 6 additions & 1 deletion src/cmd/compile/internal/ssa/rewrite386.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions src/cmd/compile/internal/ssa/rewriteAMD64.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/cmd/compile/internal/ssa/rewriteARM.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/cmd/compile/internal/ssa/rewriteARM64.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions src/cmd/compile/internal/ssa/rewriteLOONG64.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/cmd/compile/internal/ssa/rewriteMIPS.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions src/cmd/compile/internal/ssa/rewriteMIPS64.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions src/cmd/compile/internal/ssa/rewritePPC64.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions src/cmd/compile/internal/ssa/rewriteRISCV64.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions src/cmd/compile/internal/ssa/rewriteS390X.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/cmd/compile/internal/ssa/rewriteWasm.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 21f4340

Please sign in to comment.