[PATCH] D149260: [AArch64] Emit FNMADD instead of FNEG(FMADD)

[Matt Devereau via Phabricator via llvm-commits]

MattDevereau added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:5437
+  case AArch64::FNEGDr:
+    Found |= Match(AArch64::FMADDDrrr, MachineCombinerPattern::FNMADDD);
+    break;
----------------
david-arm wrote:
> Shouldn't we also be doing FMSUB here too, to ensure we are also requiring the nsz flag for the existing fnmsub combine?
I don't think FNEG(FMSUB) and FNMSUB are equivalent, so we can't assume FNMSUB from a FNEG perspective like we can with FNMAD:

FNMSUB = (a * b) - c                    //e.g. (12 * 4) - 8 = 32
FNEG(FMSUB) = -(-(a*b) + c)        //e.g. -(12 * 4) + 8 = 40


================
Comment at: llvm/test/CodeGen/AArch64/aarch64_fnmadd.ll:86
+
+define dso_local void @neg_fnmadds_contract(ptr %a, ptr %b, ptr %c) {
+; CHECK-LABEL: neg_fnmadds_contract:
----------------
david-arm wrote:
> nit: This is just a minor thing, but at first I though `@neg` referred to negation here, given the combine involves a `fneg`. Perhaps it's less confusing to just remove it and add a comment above the negative test cases explaining why the combine doesn't happen?
Done, I agree it was pretty confusing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149260