[PATCH] D38378: [ARM] Optimize {s,u}{add,sub}.with.overflow.

[Joel Galenson via Phabricator via llvm-commits]

jgalenson added a comment.

Are there any more comments, or does this look good?



================
Comment at: lib/Target/ARM/ARMBaseInstrInfo.cpp:2914
+  // We heuristically only look at the instruction immediately following MI to
+  // avoid potentially searching the entire basic block.
+  if (isPredicated(MI))
----------------
I initially decided to look only at the preceding instruction to avoid the potential quadratic behavior of scanning the whole block.  I just now gathered some data by compiling a few random files with this and a version that walks as far back as it can to look for a compare.  In a few cases that allows us to optimize slightly more compares, but not many, and in most cases there's no difference.  Thus given the extra compile time overhead and the chance that it will prevent us from sinking instructions we want to sink, keeping this heuristic for now seems good to me.  Does that seem reasonable?


https://reviews.llvm.org/D38378