[PATCH] D52463: [ARM] Fix for PR39060

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Thanks for clarifying! Looks good, with only a nit.



================
Comment at: lib/Target/ARM/ARMCodeGenPrepare.cpp:274
+  // %sub = -2 == FF FF FF FE == 4294967294
+  // So the unsigned compares (i8 and i32) would not yield the same result.
+  //
----------------
This example plugs in numbers to show that evaluating this in different types gives different results. Perhaps a quick example too how we calculate that overflow might happen changing the result: 

   %a - 2 <= 254
   %a <= 254 + 2
   %a <= 256

we find that 256 does not fit in i8, and therefore we say this is not safe to promote.

And perhaps a note somewhere that this could all be generalised and we could be accepting the general case:

  %a - %b <= %c

if we have (accurate) value range analysis available and a, b, or c are variables; I have never look at valuerange in LLVM though, and of course this definitely something for a follow up.


https://reviews.llvm.org/D52463