[PATCH] D154980: [RISCV] Don't fold vmerge into ops if fp exception can be raised

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 12 17:15:30 PDT 2023


reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM w/comments.

p.s. For the record, we can infer safety in a bunch more cases, but this doesn't matter for anything but fp.strict code.  (i.e. not the default.)



================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:3348
 
-  auto IsNoFPExcept = [this](SDValue N) {
-    return !this->mayRaiseFPException(N.getNode()) ||
-           N->getFlags().hasNoFPExcept();
-  };
-
   // Allow the peephole for non-exception True with VLMAX vector length, since
   // all the values after VL of N are dependent on Merge. VLMAX should be
----------------
Can you reword this comment?  It no longer seems to apply to the code it's adjacent to.  


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:3357
+  // affect how fflags is set.
+  if (TrueVL != VL || !IsMasked)
+    if (mayRaiseFPException(True.getNode()) &&
----------------
Slight tweak in wording: can't raise any observable fp exceptions

doesn't -> can't - a bound on the actual behavior, not the actual behavior

observable - i.e. non strict is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154980



More information about the llvm-commits mailing list