[AMDGPU] InstCombine results in performance drop in ROCM's rocRAND library in MI100

[vg0204]

Instruction folding done in #94887 causes performance drop in the benchmark testcase for normal-double distribution for mt19937 randomizer engine. {in AMD MI100 device }

The expected result should be :

benchmark_rocrand_generate
rocRAND: 300100 Runtime: 60241133 Device: AMD Instinct MI100

mt19937:
  normal-double:
      Throughput =  264.408 GB/s, Samples =   33.051 GSample/s, AvgTime (1 trial) =    3.782 ms, Time (all) =   75.641 ms, Size = 134217728

But, the mentioned PR causing drop in throughput as follows :

benchmark_rocrand_generate
rocRAND: 300100 Runtime: 60241133 Device: AMD Instinct MI100

mt19937:
  normal-double:
      Throughput =  248.093 GB/s, Samples =   31.012 GSample/s, AvgTime (1 trial) =    4.031 ms, Time (all) =   80.615 ms, Size = 134217728

[vg0204]

@c8ef, as the instruction folding is being performed for an exponent-based instruction, it supposedly should be done only if approxFunc bit of fastMathFlag is set I feel. This solved the performance drop problem as I tried it locally for mentioned specific testCase. Will do it for entire benchmark test suite & will let you know!

[llvmbot]

@llvm/issue-subscribers-backend-amdgpu

Author: Vikash Gupta (vg0204)

Instruction folding done in #94887 causes performance drop in the benchmark testcase for normal-doule for mt19937 engine. {in AMD MI100 device }

The expected result should be :

benchmark_rocrand_generate
rocRAND: 300100 Runtime: 60241133 Device: AMD Instinct MI100

mt19937:
  normal-double:
      Throughput =  264.408 GB/s, Samples =   33.051 GSample/s, AvgTime (1 trial) =    3.782 ms, Time (all) =   75.641 ms, Size = 134217728

But, the mentioned PR causing drop in throughput as follows :

benchmark_rocrand_generate
rocRAND: 300100 Runtime: 60241133 Device: AMD Instinct MI100

mt19937:
  normal-double:
      Throughput =  248.093 GB/s, Samples =   31.012 GSample/s, AvgTime (1 trial) =    4.031 ms, Time (all) =   80.615 ms, Size = 134217728

[arsenm]

@c8ef, as the instruction folding is being performed for an exponent-based instruction, it supposedly should be done only if approxFunc bit of fastMathFlag is set I feel.

No, this does not require additional flags. It is an exact transformation (that only has the avoided overflow reassoc issue)

This solved the performance drop problem as I tried it locally for mentioned specific testCase. Will do it for entire benchmark test suite & will let you know!

This should be exact and always faster. I doubt this transform is directly responsible for the performance loss and more investigation is needed. Do you have IR and ISA diffs?

[vg0204]

@c8ef, as the instruction folding is being performed for an exponent-based instruction, it supposedly should be done only if approxFunc bit of fastMathFlag is set I feel.

No, this does not require additional flags. It is an exact transformation (that only has the avoided overflow reassoc issue)

This solved the performance drop problem as I tried it locally for mentioned specific testCase. Will do it for entire benchmark test suite & will let you know!

This should be exact and always faster. I doubt this transform is directly responsible for the performance loss and more investigation is needed. Do you have IR and ISA diffs?

Working on it to extract it.

[vg0204]

The mainly observed IR pattern difference (immediately after instcombine pass incorporating mentioned intruction folding) is as follows :

Without the Idexp folding.

%30 = tail call { double, i32 } @llvm.frexp.f64.i32(double %add.i.i.i)
%31 = extractvalue { double, i32 } %30, 0
%32 = fcmp olt double %31, 0x3FE5555555555555
%33 = zext i1 %32 to i32
%34 = tail call double @llvm.ldexp.f64.i32(double %31, i32 %33)
%35 = extractvalue { double, i32 } %30, 1
%36 = sub nsw i32 %35, %33

With Idexp folding.

%30 = tail call { double, i32 } @llvm.frexp.f64.i32(double %add.i.i.i)
%31 = extractvalue { double, i32 } %30, 0
%32 = fcmp olt double %31, 0x3FE5555555555555
%.neg.i = sext i1 %32 to i32
%33 = select i1 %32, double 2.000000e+00, double 1.000000e+00
%34 = fmul double %31, %33
%35 = extractvalue { double, i32 } %30, 1
%36 = add i32 %35, %.neg.i

And these are mathematically equivalent.

[vg0204]

As for the final ISA difference in assembly file, while those ldexp not folded gets translated into AMD's v_ldexp instruction, the others got folded translates into bunch of instruction along the lines of above mentioned IR.

Could it the case that the machine's specialized (AMDGPU) v_ldexp might perform better than folding llvm@ldexp and translating the lowered IR into machine instruction?

[arsenm]

As for the final ISA difference in assembly file, while those ldexp not folded gets translated into AMD's v_ldexp instruction, the others got folded translates into bunch of instruction along the lines of above mentioned IR.

Post the diff of the actual ISA, preferably with IR you can also use to reproduce it

Could it the case that the machine's specialized (AMDGPU) v_ldexp might perform better than folding llvm@ldexp and translating the lowered IR into machine instruction?

ldexp is always the same number of cycles as fmul, and the select is also as cheap as the original extend

[vg0204]

Attached zip file contain IR immediately after instCombine pass for both with & without ldexp folding, & the diff between the final assembly.
fail-after-instcombine.zip
FinalAssemblyDiff.zip
pass-after-instcombine.zip

As for the hipcc command used to generate IR & assembly is as follow :

/opt/rocm/llvm/bin/clang++ -Xclang -fallow-half-arguments-and-returns -D__HIP_HCC_COMPAT_MODE__=1 --offload-arch=gfx908:xnack- -DUSE_PROF_API=1 -D__HIP_PLATFORM_AMD__=1 -Drocrand_EXPORTS -I/work/downstream/rocRAND/library/include -I/work/downstream/rocRAND/build-pass/library/include -Wall -Wextra -O3 -DNDEBUG -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -x hip -std=c++17 -c /work/downstream/rocRAND/library/src/rng/generator_type_mt19937.cpp

I tried various combination of given flags with omitting parts needed to generate assembly from pre-generated IR, but unable to do, so if you could have a look at it once.

[vg0204]

ping @arsenm

[arsenm]

ping @arsenm

It would be more helpful if you posted full the full IR not the -print-before (which adds the comments, and missing the global pieces needed for compilable IR), and the full ISA (not the diff)

In this case I think I see the issue in the ISA diff. The critical change is _ZN12rocrand_impl4hostL20generate_long_kernelINS0_23default_config_providerIL16r (and one other kernel) happened to go from 128 VGPRs to 129 VGPRs, so the occupancy dropped. These kernels probably should just be using an explicit occupancy bounds.

However I do see the codegen is worse in the f64 case, due to how the select gets broken up. I think we need to undo this transform in codegen, for f64. https://godbolt.org/z/Eb5csW35j

This is essentially a non-constant variant of 29fff3e. The 32-bit select is cheaper than the 64-bit select (and 64-bit constants) both of which get split up

[vg0204]

ping @arsenm

It would be more helpful if you posted full the full IR not the -print-before (which adds the comments, and missing the global pieces needed for compilable IR), and the full ISA (not the diff)

You mean the IR after every pass that transforms it?

In this case I think I see the issue in the ISA diff. The critical change is _ZN12rocrand_impl4hostL20generate_long_kernelINS0_23default_config_providerIL16r (and one other kernel) happened to go from 128 VGPRs to 129 VGPRs, so the occupancy dropped. These kernels probably should just be using an explicit occupancy bounds.

However I do see the codegen is worse in the f64 case, due to how the select gets broken up. I think we need to undo this transform in codegen, for f64. https://godbolt.org/z/Eb5csW35j

I see it!

This is essentially a non-constant variant of 29fff3e. The 32-bit select is cheaper than the 64-bit select (and 64-bit constants) both of which get split up