[PATCH] D128310: [RISCV] Move vfma_vl+fneg_vl matching to DAG combine.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8480
+static unsigned negateFMAOpcode(unsigned Opcode, bool NegMul, bool NegAcc) {
+  if (NegMul) {
+    // clang-format off
----------------
reames wrote:
> The logic flow here is very confusing.  In particular, I can't quite tell if this handles the case where we invert both or not.
> 
> I'd suggest structuring this with an explicit if (NegMul && NegAcc) case, and then use early return.
> 
> As a further idea, what we really have here is for each case, pairs of inverted opcodes.  Grouping the code that way would help readability.
> The logic flow here is very confusing.  In particular, I can't quite tell if this handles the case where we invert both or not.

The first switch updates `Opcode` so the second switch will see the change.

> 
> I'd suggest structuring this with an explicit if (NegMul && NegAcc) case, and then use early return.
> 
> As a further idea, what we really have here is for each case, pairs of inverted opcodes.  Grouping the code that way would help readability.

Is this an ask to reorder the cases in the switch so they are grouped in pairs or something else?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128310