[PATCH] D159069: [builtins] Fix signed integer overflows in fp_fixint_impl.inc

[Karl-Johan Karlsson via Phabricator via llvm-commits]

Ka-Ka added inline comments.


================
Comment at: compiler-rt/test/builtins/Unit/fixsfdi_test.c:73
 
-   return 0;
+    if (test__fixsfdi(-0x8000000000000000.0p+0F, 0x8000000000000000LL))
+      return 1;
----------------
MaskRay wrote:
> Ka-Ka wrote:
> > MaskRay wrote:
> > > Ka-Ka wrote:
> > > > The added test pass when compiling with gcc -lgcc.
> > > > 
> > > This checks the `__fixunssfdi` code path, not `fp_fixint_impl.inc`.
> > It turn out that the added test only trigger the intended case if compiler-rt is compiled with `-D__SOFTFP__` (which we do in our out of tree target). Maybe it is impossible to trigger the signed overflow in fp_fixint_impl.inc in a in tree target (ARM seems to have a feature for softfp but probably not enabled when compiling compiler-rt?).
> > Should I simply remove the added test from the patch as it don't trigger the intended case?
> > 
> should be fine to keep it...
Sure, then I keep it. I will write something in the commit message about it.

Thanks for reviewing.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159069