Skip to content

Commit cc2a5cf

Browse files
committed
cmd/compile,cmd/internal/obj/ppc64: fix some shift rules due to a regression
A recent change to improve shifts was generating some invalid cases when the rule was based on an AND. The extended mnemonics CLRLSLDI and CLRLSLWI only allow certain values for the operands and in the mask case those values were not being checked properly. This adds a check to those rules to verify that the 'b' and 'n' values used when an AND was part of the rule have correct values. There was a bug in some diag messages in asm9. The message expected 3 values but only provided 2. Those are corrected here also. The test/codegen/shift.go was updated to add a few more cases to check for the case mentioned here. Some of the comments that mention the order of operands in these extended mnemonics were wrong and those have been corrected. Fixes #41683. Change-Id: If5bb860acaa5051b9e0cd80784b2868b85898c31 Reviewed-on: https://go-review.googlesource.com/c/go/+/258138 Run-TryBot: Lynn Boger <[email protected]> Reviewed-by: Paul Murphy <[email protected]> Reviewed-by: Carlos Eduardo Seo <[email protected]> TryBot-Result: Go Bot <[email protected]> Trust: Lynn Boger <[email protected]>
1 parent 4ad5dd6 commit cc2a5cf

File tree

7 files changed

+50
-65
lines changed

7 files changed

+50
-65
lines changed

src/cmd/asm/internal/asm/testdata/ppc64enc.s

+2-2
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,8 @@ TEXT asmtest(SB),DUPOK|NOSPLIT,$0
287287
RLDICRCC $0, R4, $15, R6 // 788603c5
288288
RLDIC $0, R4, $15, R6 // 788603c8
289289
RLDICCC $0, R4, $15, R6 // 788603c9
290-
CLRLSLWI $16, R5, $8, R4 // 54a4861e
291-
CLRLSLDI $2, R4, $24, R3 // 78831588
290+
CLRLSLWI $8, R5, $6, R4 // 54a430b2
291+
CLRLSLDI $24, R4, $4, R3 // 78832508
292292

293293
BEQ 0(PC) // 41820000
294294
BGE 0(PC) // 40800000

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -570,9 +570,9 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
570570
r1 := v.Args[0].Reg()
571571
shifts := v.AuxInt
572572
p := s.Prog(v.Op.Asm())
573-
// clrlslwi ra,rs,sh,mb will become rlwinm ra,rs,sh,mb-sh,31-n as described in ISA
574-
p.From = obj.Addr{Type: obj.TYPE_CONST, Offset: ssa.GetPPC64Shiftsh(shifts)}
575-
p.SetFrom3(obj.Addr{Type: obj.TYPE_CONST, Offset: ssa.GetPPC64Shiftmb(shifts)})
573+
// clrlslwi ra,rs,mb,sh will become rlwinm ra,rs,sh,mb-sh,31-sh as described in ISA
574+
p.From = obj.Addr{Type: obj.TYPE_CONST, Offset: ssa.GetPPC64Shiftmb(shifts)}
575+
p.SetFrom3(obj.Addr{Type: obj.TYPE_CONST, Offset: ssa.GetPPC64Shiftsh(shifts)})
576576
p.Reg = r1
577577
p.To.Type = obj.TYPE_REG
578578
p.To.Reg = r
@@ -582,9 +582,9 @@ func ssaGenValue(s *gc.SSAGenState, v *ssa.Value) {
582582
r1 := v.Args[0].Reg()
583583
shifts := v.AuxInt
584584
p := s.Prog(v.Op.Asm())
585-
// clrlsldi ra,rs,sh,mb will become rldic ra,rs,sh,mb-sh
586-
p.From = obj.Addr{Type: obj.TYPE_CONST, Offset: ssa.GetPPC64Shiftsh(shifts)}
587-
p.SetFrom3(obj.Addr{Type: obj.TYPE_CONST, Offset: ssa.GetPPC64Shiftmb(shifts)})
585+
// clrlsldi ra,rs,mb,sh will become rldic ra,rs,sh,mb-sh
586+
p.From = obj.Addr{Type: obj.TYPE_CONST, Offset: ssa.GetPPC64Shiftmb(shifts)}
587+
p.SetFrom3(obj.Addr{Type: obj.TYPE_CONST, Offset: ssa.GetPPC64Shiftsh(shifts)})
588588
p.Reg = r1
589589
p.To.Type = obj.TYPE_REG
590590
p.To.Reg = r

src/cmd/compile/internal/ssa/gen/PPC64.rules

+4-5
Original file line numberDiff line numberDiff line change
@@ -1018,13 +1018,12 @@
10181018
(SLDconst [c] z:(MOVHZreg x)) && c < 16 && z.Uses == 1 => (CLRLSLDI [newPPC64ShiftAuxInt(c,48,63,64)] x)
10191019
(SLDconst [c] z:(MOVWZreg x)) && c < 32 && z.Uses == 1 => (CLRLSLDI [newPPC64ShiftAuxInt(c,32,63,64)] x)
10201020

1021-
(SLDconst [c] z:(ANDconst [d] x)) && z.Uses == 1 && isPPC64ValidShiftMask(d) => (CLRLSLDI [newPPC64ShiftAuxInt(c,64-getPPC64ShiftMaskLength(d),63,64)] x)
1022-
(SLDconst [c] z:(AND (MOVDconst [d]) x)) && z.Uses == 1 && isPPC64ValidShiftMask(d) => (CLRLSLDI [newPPC64ShiftAuxInt(c,64-getPPC64ShiftMaskLength(d),63,64)] x)
1021+
(SLDconst [c] z:(ANDconst [d] x)) && z.Uses == 1 && isPPC64ValidShiftMask(d) && c <= (64-getPPC64ShiftMaskLength(d)) => (CLRLSLDI [newPPC64ShiftAuxInt(c,64-getPPC64ShiftMaskLength(d),63,64)] x)
1022+
(SLDconst [c] z:(AND (MOVDconst [d]) x)) && z.Uses == 1 && isPPC64ValidShiftMask(d) && c<=(64-getPPC64ShiftMaskLength(d)) => (CLRLSLDI [newPPC64ShiftAuxInt(c,64-getPPC64ShiftMaskLength(d),63,64)] x)
10231023
(SLWconst [c] z:(MOVBZreg x)) && z.Uses == 1 && c < 8 => (CLRLSLWI [newPPC64ShiftAuxInt(c,24,31,32)] x)
10241024
(SLWconst [c] z:(MOVHZreg x)) && z.Uses == 1 && c < 16 => (CLRLSLWI [newPPC64ShiftAuxInt(c,16,31,32)] x)
1025-
(SLWconst [c] z:(MOVWZreg x)) && z.Uses == 1 && c < 24 => (CLRLSLWI [newPPC64ShiftAuxInt(c,8,31,32)] x)
1026-
(SLWconst [c] z:(ANDconst [d] x)) && z.Uses == 1 && isPPC64ValidShiftMask(d) => (CLRLSLWI [newPPC64ShiftAuxInt(c,32-getPPC64ShiftMaskLength(d),31,32)] x)
1027-
(SLWconst [c] z:(AND (MOVDconst [d]) x)) && z.Uses == 1 && isPPC64ValidShiftMask(d) => (CLRLSLWI [newPPC64ShiftAuxInt(c,32-getPPC64ShiftMaskLength(d),31,32)] x)
1025+
(SLWconst [c] z:(ANDconst [d] x)) && z.Uses == 1 && isPPC64ValidShiftMask(d) && c<=(32-getPPC64ShiftMaskLength(d)) => (CLRLSLWI [newPPC64ShiftAuxInt(c,32-getPPC64ShiftMaskLength(d),31,32)] x)
1026+
(SLWconst [c] z:(AND (MOVDconst [d]) x)) && z.Uses == 1 && isPPC64ValidShiftMask(d) && c<=(32-getPPC64ShiftMaskLength(d)) => (CLRLSLWI [newPPC64ShiftAuxInt(c,32-getPPC64ShiftMaskLength(d),31,32)] x)
10281027
// special case for power9
10291028
(SL(W|D)const [c] z:(MOVWreg x)) && c < 32 && objabi.GOPPC64 >= 9 => (EXTSWSLconst [c] x)
10301029

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -1380,8 +1380,8 @@ func GetPPC64Shiftme(auxint int64) int64 {
13801380
return int64(int8(auxint))
13811381
}
13821382

1383-
// Catch the simple ones first
1384-
// TODO: Later catch more cases
1383+
// This verifies that the mask occupies the
1384+
// rightmost bits.
13851385
func isPPC64ValidShiftMask(v int64) bool {
13861386
if ((v + 1) & v) == 0 {
13871387
return true

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

+8-26
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/cmd/internal/obj/ppc64/asm9.go

+18-17
Original file line numberDiff line numberDiff line change
@@ -2749,27 +2749,27 @@ func (c *ctxt9) asmout(p *obj.Prog, o *Optab, out []uint32) {
27492749
me := int(d)
27502750
sh := c.regoff(&p.From)
27512751
if me < 0 || me > 63 || sh > 63 {
2752-
c.ctxt.Diag("Invalid me or sh for RLDICR: %x %x\n%v", int(d), sh)
2752+
c.ctxt.Diag("Invalid me or sh for RLDICR: %x %x\n%v", int(d), sh, p)
27532753
}
27542754
o1 = AOP_RLDIC(c.oprrr(p.As), uint32(p.To.Reg), uint32(r), uint32(sh), uint32(me))
27552755

27562756
case ARLDICL, ARLDICLCC, ARLDIC, ARLDICCC:
27572757
mb := int(d)
27582758
sh := c.regoff(&p.From)
27592759
if mb < 0 || mb > 63 || sh > 63 {
2760-
c.ctxt.Diag("Invalid mb or sh for RLDIC, RLDICL: %x %x\n%v", mb, sh)
2760+
c.ctxt.Diag("Invalid mb or sh for RLDIC, RLDICL: %x %x\n%v", mb, sh, p)
27612761
}
27622762
o1 = AOP_RLDIC(c.oprrr(p.As), uint32(p.To.Reg), uint32(r), uint32(sh), uint32(mb))
27632763

27642764
case ACLRLSLDI:
27652765
// This is an extended mnemonic defined in the ISA section C.8.1
2766-
// clrlsldi ra,rs,n,b --> rldic ra,rs,n,b-n
2766+
// clrlsldi ra,rs,b,n --> rldic ra,rs,n,b-n
27672767
// It maps onto RLDIC so is directly generated here based on the operands from
27682768
// the clrlsldi.
2769-
b := int(d)
2770-
n := c.regoff(&p.From)
2771-
if n > int32(b) || b > 63 {
2772-
c.ctxt.Diag("Invalid n or b for CLRLSLDI: %x %x\n%v", n, b)
2769+
n := int32(d)
2770+
b := c.regoff(&p.From)
2771+
if n > b || b > 63 {
2772+
c.ctxt.Diag("Invalid n or b for CLRLSLDI: %x %x\n%v", n, b, p)
27732773
}
27742774
o1 = AOP_RLDIC(OP_RLDIC, uint32(p.To.Reg), uint32(r), uint32(n), uint32(b)-uint32(n))
27752775

@@ -3395,14 +3395,15 @@ func (c *ctxt9) asmout(p *obj.Prog, o *Optab, out []uint32) {
33953395
v := c.regoff(&p.From)
33963396
switch p.As {
33973397
case ACLRLSLWI:
3398-
b := c.regoff(p.GetFrom3())
3398+
n := c.regoff(p.GetFrom3())
33993399
// This is an extended mnemonic described in the ISA C.8.2
3400-
// clrlslwi ra,rs,n,b -> rlwinm ra,rs,n,b-n,31-n
3400+
// clrlslwi ra,rs,b,n -> rlwinm ra,rs,n,b-n,31-n
34013401
// It maps onto rlwinm which is directly generated here.
3402-
if v < 0 || v > 32 || b > 32 {
3403-
c.ctxt.Diag("Invalid n or b for CLRLSLWI: %x %x\n%v", v, b)
3402+
if n > v || v >= 32 {
3403+
c.ctxt.Diag("Invalid n or b for CLRLSLWI: %x %x\n%v", v, n, p)
34043404
}
3405-
o1 = OP_RLW(OP_RLWINM, uint32(p.To.Reg), uint32(p.Reg), uint32(v), uint32(b-v), uint32(31-v))
3405+
3406+
o1 = OP_RLW(OP_RLWINM, uint32(p.To.Reg), uint32(p.Reg), uint32(n), uint32(v-n), uint32(31-n))
34063407
default:
34073408
var mask [2]uint8
34083409
c.maskgen(p, mask[:], uint32(c.regoff(p.GetFrom3())))
@@ -3414,16 +3415,16 @@ func (c *ctxt9) asmout(p *obj.Prog, o *Optab, out []uint32) {
34143415
v := c.regoff(&p.From)
34153416
switch p.As {
34163417
case ACLRLSLWI:
3417-
b := c.regoff(p.GetFrom3())
3418-
if v > b || b > 32 {
3418+
n := c.regoff(p.GetFrom3())
3419+
if n > v || v >= 32 {
34193420
// Message will match operands from the ISA even though in the
34203421
// code it uses 'v'
3421-
c.ctxt.Diag("Invalid n or b for CLRLSLWI: %x %x\n%v", v, b)
3422+
c.ctxt.Diag("Invalid n or b for CLRLSLWI: %x %x\n%v", v, n, p)
34223423
}
34233424
// This is an extended mnemonic described in the ISA C.8.2
3424-
// clrlslwi ra,rs,n,b -> rlwinm ra,rs,n,b-n,31-n
3425+
// clrlslwi ra,rs,b,n -> rlwinm ra,rs,n,b-n,31-n
34253426
// It generates the rlwinm directly here.
3426-
o1 = OP_RLW(OP_RLWINM, uint32(p.To.Reg), uint32(p.Reg), uint32(v), uint32(b-v), uint32(31-v))
3427+
o1 = OP_RLW(OP_RLWINM, uint32(p.To.Reg), uint32(p.Reg), uint32(n), uint32(v-n), uint32(31-n))
34273428
default:
34283429
var mask [2]uint8
34293430
c.maskgen(p, mask[:], uint32(c.regoff(p.GetFrom3())))

test/codegen/shift.go

+10-7
Original file line numberDiff line numberDiff line change
@@ -187,21 +187,24 @@ func checkCombinedShifts(v8 uint8, v16 uint16, v32 uint32, x32 int32, v64 uint64
187187
// ppc64le:-"AND","CLRLSLWI"
188188
// ppc64:-"AND","CLRLSLWI"
189189
f := (v8 &0xF) << 2
190-
// ppc64le:-"AND","CLRLSLWI"
191-
// ppc64:-"AND","CLRLSLWI"
190+
// ppc64le:"CLRLSLWI"
191+
// ppc64:"CLRLSLWI"
192192
f += byte(v16)<<3
193193
// ppc64le:-"AND","CLRLSLWI"
194194
// ppc64:-"AND","CLRLSLWI"
195195
g := (v16 & 0xFF) << 3
196196
// ppc64le:-"AND","CLRLSLWI"
197197
// ppc64:-"AND","CLRLSLWI"
198198
h := (v32 & 0xFFFFF) << 2
199-
// ppc64le:-"AND","CLRLSLWI"
200-
// ppc64:-"AND","CLRLSLWI"
201-
h += uint32(v64)<<4
202-
// ppc64le:-"AND","CLRLSLDI"
203-
// ppc64:-"AND","CLRLSLDI"
199+
// ppc64le:"CLRLSLDI"
200+
// ppc64:"CLRLSLDI"
204201
i := (v64 & 0xFFFFFFFF) << 5
202+
// ppc64le:-"CLRLSLDI"
203+
// ppc64:-"CLRLSLDI"
204+
i += (v64 & 0xFFFFFFF) << 38
205+
// ppc64le/power9:-"CLRLSLDI"
206+
// ppc64/power9:-"CLRLSLDI"
207+
i += (v64 & 0xFFFF00) << 10
205208
// ppc64le/power9:-"SLD","EXTSWSLI"
206209
// ppc64/power9:-"SLD","EXTSWSLI"
207210
j := int64(x32+32)*8

0 commit comments

Comments
 (0)