[PATCH] D130508: [RISCV] Teach translateSetCCForBranch to help improve constant materialization.

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1417
+      // Or X > C to C >= C+1 if it could use LUI.
+      if (C == -1 ||
+          (!isInt<12>(C) && isInt<12>(C + 1)) ||
----------------
craig.topper wrote:
> reames wrote:
> > Can't we generalize this to be if RISCVMatInt::getIntMatCost(C+1) < RISCVMatInt::getIntMatCost(C)?
> I was trying to be a little conservative if the constant required multiple instructions and was used by other instructions. Changing it could introduce multiple slightly different constants. I figured creating an LUI or ADDI wasn't as bad.
> 
> Maybe I should check one use and/or opaque constant?
I'd be fine with either the one use check, or a comment that says there deliberately isn't a one-use check and that's why this is restricted to the single instruction forms.

I think you could also restrict the single instruction case in terms of the getIntMatCost interface too, but that's not required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130508