[PATCH] D15315: [X86ISelLowering] Replace uint64_t with APInt to prevent overflow

Chen Li via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 10 08:50:43 PST 2015


chenli added a comment.

I applied the patch locally and it worked with 0xFFFFFFFF. However, in practice, neither 0xFFFFFFFF nor 0 will flow into this function because llvm handles them with special cases. For 0, it returns a 0 directly, and for 0xFFFFFFFF, it returns 0 - (the other operand). I've hacked to bypass the special cases in order to test the overflow case could be handled correctly by this code.

In http://reviews.llvm.org/D15315#307134, @RKSimon wrote:

> In http://reviews.llvm.org/D15315#306841, @chenli wrote:
>
> > I applied the patch locally and it worked with 0xFFFFFFFF. However, in practice, neither 0xFFFFFFFF nor 0 will flow into this function because llvm handles them with special cases. For 0, it returns a 0 directly, and for 0xFFFFFFFF, it returns 0 - (the other operand). I've hacked to bypass the special cases in order to test the overflow case could be handled correctly by this code.
>
>
> If that is true - might it be better just to add suitable asserts / early returns to http://reviews.llvm.org/D14603 that check for cases that would overflow. In which case, I feel rather guilty as the APInt conversion work is probably unnecessary.....


Sure. I will discard this patch and update http://reviews.llvm.org/D14603.


http://reviews.llvm.org/D15315





More information about the llvm-commits mailing list