gonum / blas

A BLAS implementation for Go [DEPRECATED]
172 stars 16 forks source link

native: Use asm.Dscal in Dscal #150

Closed vladimir-ch closed 8 years ago

vladimir-ch commented 8 years ago

I wonder if we shouldn't keep the PosInc case as Go loop. Also, mat64.Vector does not allow negative inc, BLAS does nothing when negative inc, so we could safely drop the last argument from the assembler routines.

% benchstat before.txt after.txt 
name                   old time/op  new time/op  delta
DscalSmallUnitaryInc   12.6ns ± 0%   9.4ns ± 0%  -25.26%  (p=0.029 n=4+4)
DscalSmallPosInc       13.4ns ± 0%  13.2ns ± 0%   -1.49%  (p=0.029 n=4+4)
DscalMediumUnitaryInc   690ns ± 0%   164ns ± 0%  -76.25%  (p=0.029 n=4+4)
DscalMediumPosInc      1.14µs ± 0%  1.15µs ± 0%   +1.36%  (p=0.029 n=4+4)
DscalLargeUnitaryInc   71.1µs ± 0%  42.8µs ± 0%  -39.87%  (p=0.029 n=4+4)
DscalLargePosInc        197µs ± 0%   183µs ± 1%   -7.44%  (p=0.029 n=4+4)
DscalHugeUnitaryInc    9.92ms ± 1%  8.00ms ± 1%  -19.38%  (p=0.029 n=4+4)
DscalHugePosInc        41.4ms ± 0%  41.1ms ± 0%   -0.76%  (p=0.029 n=4+4)
kortschak commented 8 years ago

I wonder if we shouldn't keep the PosInc case as Go loop.

You mean here?

Also, mat64.Vector does not allow negative inc, BLAS does nothing when negative inc, so we could safely drop the last argument from the assembler routines.

I don't understand this.

vladimir-ch commented 8 years ago

I wonder if we shouldn't keep the PosInc case as Go loop.

You mean here?

I mean dropping the commit 8b1fdd7. The benchmark does not show much of a benefit and we already know that the asm.DscalInc routines are a bit eccentric.

Also, mat64.Vector does not allow negative inc, BLAS does nothing when negative inc, so we could safely drop the last argument from the assembler routines.

I don't understand this.

Since the increment passed to asm.Dscal routines will be always positive, the ix and idst arguments in

func DscalInc(alpha float64, x []float64, n, incX, ix uintptr)
func DscalIncTo(dst []float64, incDst, idst uintptr, alpha float64, x []float64, n, incX, ix uintptr)

will be always zero. We could remove them and simplify the signature a bit.

kortschak commented 8 years ago

Clear now thanks.

Both those SGTM. The second should add a comment that increments are always positive - we use them that way now, but we need it documented at the site.

vladimir-ch commented 8 years ago

Commit dropped. PTAL @kortschak

vladimir-ch commented 8 years ago

ping

kortschak commented 8 years ago

Sorry, I thought I had done this.

LGTM