[PATCH] D54205: [RISCV] Add support for the various RISC-V FMA instruction variants

[Alex Bradbury via Phabricator via llvm-commits]

asb requested changes to this revision.
asb added a comment.
This revision now requires changes to proceed.

Just some minor tweaks and I think this is good to go. In addition to the changes mentioned in inline comments, please:

- add `nounwind` to all the test functions
- Remove `tail` from the calls to llvm.fma as it doesn't impact the generated code (I have a preference to remove everything that doesn't impact the generated code).

Thanks!

Given that the names of the FMA instructions have been a source of great confusion, I sanity checked vs gcc:

  $ cat fma.c 
  float fmadd(float a, float b, float c) {
    return a*b+c;
  }
  
  float fmsub(float a, float b, float c) {
    return a*b-c;
  }
  
  float fnmsub(float a, float b, float c) {
    return -a*b+c;
  }
  
  float fnmadd(float a, float b, float c) {
    return -a*b-c;
  }



  $ ./riscv32-unknown-elf-gcc -march=rv32imf -mabi=ilp32f fma.c -S -O2 -o - | grep -v [[:space:]]\\.
  fmadd:
  	fmadd.s	fa0,fa0,fa1,fa2
  	ret
  fmsub:
  	fmsub.s	fa0,fa0,fa1,fa2
  	ret
  fnmsub:
  	fnmsub.s	fa0,fa0,fa1,fa2
  	ret
  fnmadd:
  	fnmadd.s	fa0,fa0,fa1,fa2
  	ret



================
Comment at: test/CodeGen/RISCV/double-intrinsics.ll:200
 
-declare double @llvm.fma.f64(double, double, double)
+declare double @llvm.fmuladd.f64(double, double, double)
 
----------------
Could you keep a test for llvm.fma in this and float-intrinsics.ll? I know it's covered by {float,double}-arith.ll, but it keeps things simpler if these -intrinsics.ll tests contain _all_ float intrinsics.


================
Comment at: test/CodeGen/RISCV/double-intrinsics.ll:202
 
-; TODO: Select RISC-V FMA instruction.
-define double @fma_f64(double %a, double %b, double %c) {
-; RV32IFD-LABEL: fma_f64:
+define double @fmulladd_f64(double %a, double %b, double %c) {
+; Use of fmadd depends on TargetLowering::isFMAFasterthanFMulAndFAdd
----------------
Should be fmuladd_f64


================
Comment at: test/CodeGen/RISCV/float-intrinsics.ll:185
 
-; TODO: Select RISC-V FMA instruction.
-define float @fma_f32(float %a, float %b, float %c) {
-; RV32IF-LABEL: fma_f32:
+define float @fmulladd_f32(float %a, float %b, float %c) {
+; Use of fmadd depends on TargetLowering::isFMAFasterthanFMulAndFAdd
----------------
Should be fmuladd_f32


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54205/new/

https://reviews.llvm.org/D54205