[PATCH] D134725: [ADT] Add support for more formats in APFixedPoint
Tyker via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 7 14:13:25 PDT 2022
Tyker added inline comments.
================
Comment at: llvm/include/llvm/ADT/APFixedPoint.h:216-217
+ (getLsbWeight() > 0) ? Val.extend(getWidth() + getLsbWeight()) : Val;
if (Val < 0 && Val != -Val) // Cover the case when we have the min val
- return -(-Val >> getScale());
- else
- return Val >> getScale();
+ return -(-ExtVal.relativeShl(getLsbWeight()));
+ return ExtVal.relativeShl(getLsbWeight());
----------------
leonardchan wrote:
> At a glance, I think the issue might be here. The 16-bit wide APSInt is -3276. Prior to this patch, the order was negate Val (3276), shift (3276 >> 15 == 0), then negate again (0). Now the updated process looks to be shift (-3276 >> 15 == -1), negate (1), then negate again (-1). Looks like the right fix is `-((-ExtVal).relativeShl(getLsbWeight()))`. @Tyker could you confirm this?
You are right, this is incorrect.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134725/new/
https://reviews.llvm.org/D134725
More information about the llvm-commits
mailing list