Skip to content

Commit 7502ed3

Browse files
committed
cmd/compile: when merging instructions, prefer line number of faulting insn
Normally this happens when combining a sign extension and a load. We want the resulting combo-instruction to get the line number of the load, not the line number of the sign extension. For each rule, compute where we should get its line number by finding a value on the match side that can fault. Use that line number for all the new values created on the right-hand side. Fixes #27201 Change-Id: I19b3c6f468fff1a3c0bfbce2d6581828557064a3 Reviewed-on: https://go-review.googlesource.com/c/156937 Reviewed-by: David Chase <[email protected]>
1 parent 70931c0 commit 7502ed3

File tree

8 files changed

+693
-633
lines changed

8 files changed

+693
-633
lines changed

src/cmd/compile/internal/ssa/gen/rulegen.go

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,11 @@ func genRules(arch arch) {
209209

210210
canFail = false
211211
fmt.Fprintf(buf, "for {\n")
212-
if genMatch(buf, arch, match, rule.loc) {
212+
pos, matchCanFail := genMatch(buf, arch, match, rule.loc)
213+
if pos == "" {
214+
pos = "v.Pos"
215+
}
216+
if matchCanFail {
213217
canFail = true
214218
}
215219

@@ -221,7 +225,7 @@ func genRules(arch arch) {
221225
log.Fatalf("unconditional rule %s is followed by other rules", match)
222226
}
223227

224-
genResult(buf, arch, result, rule.loc)
228+
genResult(buf, arch, result, rule.loc, pos)
225229
if *genLog {
226230
fmt.Fprintf(buf, "logRule(\"%s\")\n", rule.loc)
227231
}
@@ -291,10 +295,11 @@ func genRules(arch arch) {
291295
_, _, _, aux, s := extract(match) // remove parens, then split
292296

293297
// check match of control value
298+
pos := ""
294299
if s[0] != "nil" {
295300
fmt.Fprintf(w, "v := b.Control\n")
296301
if strings.Contains(s[0], "(") {
297-
genMatch0(w, arch, s[0], "v", map[string]struct{}{}, false, rule.loc)
302+
pos, _ = genMatch0(w, arch, s[0], "v", map[string]struct{}{}, false, rule.loc)
298303
} else {
299304
fmt.Fprintf(w, "_ = v\n") // in case we don't use v
300305
fmt.Fprintf(w, "%s := b.Control\n", s[0])
@@ -335,7 +340,10 @@ func genRules(arch arch) {
335340
if t[0] == "nil" {
336341
fmt.Fprintf(w, "b.SetControl(nil)\n")
337342
} else {
338-
fmt.Fprintf(w, "b.SetControl(%s)\n", genResult0(w, arch, t[0], new(int), false, false, rule.loc))
343+
if pos == "" {
344+
pos = "v.Pos"
345+
}
346+
fmt.Fprintf(w, "b.SetControl(%s)\n", genResult0(w, arch, t[0], new(int), false, false, rule.loc, pos))
339347
}
340348
if aux != "" {
341349
fmt.Fprintf(w, "b.Aux = %s\n", aux)
@@ -386,15 +394,17 @@ func genRules(arch arch) {
386394
}
387395
}
388396

389-
// genMatch reports whether the match can fail.
390-
func genMatch(w io.Writer, arch arch, match string, loc string) bool {
397+
// genMatch returns the variable whose source position should be used for the
398+
// result (or "" if no opinion), and a boolean that reports whether the match can fail.
399+
func genMatch(w io.Writer, arch arch, match string, loc string) (string, bool) {
391400
return genMatch0(w, arch, match, "v", map[string]struct{}{}, true, loc)
392401
}
393402

394-
func genMatch0(w io.Writer, arch arch, match, v string, m map[string]struct{}, top bool, loc string) bool {
403+
func genMatch0(w io.Writer, arch arch, match, v string, m map[string]struct{}, top bool, loc string) (string, bool) {
395404
if match[0] != '(' || match[len(match)-1] != ')' {
396405
panic("non-compound expr in genMatch0: " + match)
397406
}
407+
pos := ""
398408
canFail := false
399409

400410
op, oparch, typ, auxint, aux, args := parseValue(match, arch, loc)
@@ -404,6 +414,10 @@ func genMatch0(w io.Writer, arch arch, match, v string, m map[string]struct{}, t
404414
fmt.Fprintf(w, "if %s.Op != Op%s%s {\nbreak\n}\n", v, oparch, op.name)
405415
canFail = true
406416
}
417+
if op.faultOnNilArg0 || op.faultOnNilArg1 {
418+
// Prefer the position of an instruction which could fault.
419+
pos = v + ".Pos"
420+
}
407421

408422
if typ != "" {
409423
if !isVariable(typ) {
@@ -494,7 +508,16 @@ func genMatch0(w io.Writer, arch arch, match, v string, m map[string]struct{}, t
494508
argname = fmt.Sprintf("%s_%d", v, i)
495509
}
496510
fmt.Fprintf(w, "%s := %s.Args[%d]\n", argname, v, i)
497-
if genMatch0(w, arch, arg, argname, m, false, loc) {
511+
argPos, argCanFail := genMatch0(w, arch, arg, argname, m, false, loc)
512+
if argPos != "" {
513+
// Keep the argument in preference to the parent, as the
514+
// argument is normally earlier in program flow.
515+
// Keep the argument in preference to an earlier argument,
516+
// as that prefers the memory argument which is also earlier
517+
// in the program flow.
518+
pos = argPos
519+
}
520+
if argCanFail {
498521
canFail = true
499522
}
500523
}
@@ -503,10 +526,10 @@ func genMatch0(w io.Writer, arch arch, match, v string, m map[string]struct{}, t
503526
fmt.Fprintf(w, "if len(%s.Args) != %d {\nbreak\n}\n", v, len(args))
504527
canFail = true
505528
}
506-
return canFail
529+
return pos, canFail
507530
}
508531

509-
func genResult(w io.Writer, arch arch, result string, loc string) {
532+
func genResult(w io.Writer, arch arch, result string, loc string, pos string) {
510533
move := false
511534
if result[0] == '@' {
512535
// parse @block directive
@@ -515,9 +538,9 @@ func genResult(w io.Writer, arch arch, result string, loc string) {
515538
result = s[1]
516539
move = true
517540
}
518-
genResult0(w, arch, result, new(int), true, move, loc)
541+
genResult0(w, arch, result, new(int), true, move, loc, pos)
519542
}
520-
func genResult0(w io.Writer, arch arch, result string, alloc *int, top, move bool, loc string) string {
543+
func genResult0(w io.Writer, arch arch, result string, alloc *int, top, move bool, loc string, pos string) string {
521544
// TODO: when generating a constant result, use f.constVal to avoid
522545
// introducing copies just to clean them up again.
523546
if result[0] != '(' {
@@ -554,7 +577,7 @@ func genResult0(w io.Writer, arch arch, result string, alloc *int, top, move boo
554577
}
555578
v = fmt.Sprintf("v%d", *alloc)
556579
*alloc++
557-
fmt.Fprintf(w, "%s := b.NewValue0(v.Pos, Op%s%s, %s)\n", v, oparch, op.name, typ)
580+
fmt.Fprintf(w, "%s := b.NewValue0(%s, Op%s%s, %s)\n", v, pos, oparch, op.name, typ)
558581
if move && top {
559582
// Rewrite original into a copy
560583
fmt.Fprintf(w, "v.reset(OpCopy)\n")
@@ -569,7 +592,7 @@ func genResult0(w io.Writer, arch arch, result string, alloc *int, top, move boo
569592
fmt.Fprintf(w, "%s.Aux = %s\n", v, aux)
570593
}
571594
for _, arg := range args {
572-
x := genResult0(w, arch, arg, alloc, false, move, loc)
595+
x := genResult0(w, arch, arg, alloc, false, move, loc, pos)
573596
fmt.Fprintf(w, "%s.AddArg(%s)\n", v, x)
574597
}
575598

src/cmd/compile/internal/ssa/rewrite386.go

Lines changed: 16 additions & 16 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)