Differential D33836
[AArch64] Enable FeatureFuseAES for the generic processor model. Authored by fhahn on Jun 2 2017, 8:38 AM.
Details Scheduling AESE/AESMC and AESD/AESIMC instruction pairs back-to-back This change should not have a major performance impact on processors
Diff Detail Event TimelineComment Actions Makes sense to me. But now that "generic" is the default, it'll impact all cores equally, so I'll let other people comment before approving. Comment Actions Since it follows the precedent in GCC, I am fine with it. I suspect that targets that do not support this, whether in order or out of order, would perform the same. Can you share any data, @fhahn? Comment Actions The performance impact on cores without that optimization should be minimal, but unfortunately I only have access to Cortex-A cores for benchmarking, so I cannot provide any numbers for other cores. On early Cortex-A cores the impact was not noticeable though.
Comment Actions I can probably scare up whether this impacts ThunderX processors. What benchmark should I look at? OpenSSL?
Comment Actions Update CHECK- line. @evandro I’m sorry, I cannot share the exact details for various reasons, but it was over 40% on a proprietary benchmark. @joelkevinjones unfortunately I cannot share the benchmark we used either, and I’m not aware of a publicly-available one. Comment Actions I understand that, but perhaps you could have a simple loop with AESE/AESMC and another with AESD/AESIMC and report their throughput on A53 and A57 before and after the patch. As a matter of fact, I encourage you to consider adding such a test to LNT. Comment Actions I understand that the case for enabling FeatureFuseAES is quite weak at the moment! I'll see if I can find any public benchmarks, otherwise I'll try and add a handwritten benchmark to LNT. Comment Actions I had another look and it turns out I can’t add a microbenchmark for legal reasons. Unfortunately the only thing I can say is that back-to-back scheduling brings double digit performance improvements for code making heavy use of AES instructions on Cortex-A CPUs and refer to the public software optimization guides for Cortex-A72 and Cortex-A57, which both encourage this optimization. Comment Actions My two cents. I think this is likely to give gains where pipeline leverages it, and unlikely to cause regressions otherwise. Comment Actions We already have the IR in misched-fusion-aes.ll that can be used for microbenchmarks (just call the functions in a loop)? I think that should be theoretically enough for an evaluation. Cheers,
Comment Actions At a higher level this makes sense to me. Apple targets default to cyclone so they should not be affected. Comment Actions Thanks for all the comments! Unless there are any objections raised, I'll commit the change tomorrow. | ||||||||||||||||||
I don't think that you should change CHECKCORTEX.