[PATCH] D71992: [ARM] Unrestrict Armv8 IT blocks

Wed Jan 15 06:57:59 PST 2020

samparker marked an inline comment as done.
samparker added a comment.

> Are there any cores where we don't want FeatureDontRestrictIT?

How would you feel about unrestricting by default..? One of the GCC guys I was talking too also said it's hampering performance compared to armv7. I've got little bandwidth and I want to get some more numbers, and I'm not quite trusting my chromebook at the minute... but its' A72/A53 and I'm seeing good improvements on my data set of 1 (coremark) of 14% improvement.

================
Comment at: llvm/lib/Target/ARM/ARMFeatures.h:77
 // there are some "conditionally deprecated" opcodes
   case ARM::tADDspr:
   case ARM::tBLXr:
----------------
efriedma wrote:
> The checks for tADDspr and tADDrSP look wrong; neither of those instructions are deprecated, as far as I can tell. Maybe it was confused with something else?  (I guess this isn't really part of you patch; feel free to ignore.)
> 
> Are these checks intentionally not controlled by the "Restricted" boolean?
There's a note in Table F1-15 'Non-deprecated IT 16-bit conditional instructions' about using the PC, I think this logic is okay. And yes, it's intention, I've heard that some cores branch predictors don't work as well when some of these instructions are used.

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

https://reviews.llvm.org/D71992