[PATCH] D135814: [RISCV] Add basic support for the sifive-7-series short forward branch optimization.

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:9432
     APInt Mask = APInt::getBitsSetFrom(LHS.getValueSizeInBits(), 1);
-    if (isNullConstant(RHS) && ISD::isIntEqualitySetCC(CCVal) &&
-        DAG.MaskedValueIsZero(LHS, Mask)) {
+    if (!Subtarget.hasShortForwardBranchOpt() && isNullConstant(RHS) &&
+        ISD::isIntEqualitySetCC(CCVal) && DAG.MaskedValueIsZero(LHS, Mask)) {
----------------
I think you probably want to make the 0 and -1 arm transforms conditional too right?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:1773
+    WorkingMI.getOperand(3).setImm(CC);
+    return TargetInstrInfo::commuteInstructionImpl(MI, /*NewMI*/ false, OpIdx1,
+                                                   OpIdx2);
----------------
Shouldn't MI be WorkingMI here?


================
Comment at: llvm/test/CodeGen/RISCV/short-foward-branch-opt.ll:14
+; NOSFB-LABEL: test1:
+; NOSFB:       # %bb.0:
+; NOSFB-NEXT:    beqz a2, .LBB0_2
----------------
Are you intentionally giving me a punch list?  :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135814