[PATCH] D106533: [RISCV] Support simple fractional steps in matching VID sequences

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1470
+      uint64_t ExpectedVal =
+          (int64_t)(Idx * (uint64_t)*SeqStepNum) / *SeqStepDenom;
+      int64_t Addend = SignExtend64(Val - ExpectedVal, EltSizeInBits);
----------------
frasercrmck wrote:
> craig.topper wrote:
> > Why is SeqStepNum cast from int64_t to uint64_t? I don't understand why it is the original code either.
> It's very possible I went overboard in trying to prevent signed integer overflow e.g. `INT64_MIN`. I could swear the sanitizers were complaining at me though.
Good point. I guess I'm only used to thinking about addition/subtract overflow not multiply overflow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106533