[PATCH] D140747: [Transform] Rewrite LowerSwitch using APInt
Peter Rong via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 20 14:42:39 PST 2023
Peter marked an inline comment as done.
Peter added inline comments.
================
Comment at: llvm/lib/Transforms/Utils/LowerSwitch.cpp:461
- APInt N = High - Low + 1;
- assert(N.sge(SignedZero) && "Popularity shouldn't be negative.");
// Explict insert to make sure the bitwidth of APInts match
----------------
arsenm wrote:
> Peter wrote:
> > arsenm wrote:
> > > I don't understand why this wouldn't just be unsigned to begin with
> > Old version used `int64_t N`, which would have the same problem.
> Right, so why isn’t it OK to just switch to always using unsigned?
Well, old version has existed and works for years, I figure we shouldn't change that decision unless we have a good reason
And I now this (fixed) bug gave us a reason. :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140747/new/
https://reviews.llvm.org/D140747
More information about the llvm-commits
mailing list