[PATCH] D22042: [AArch64] Macro fusion of simple ALU ops with branches for Broadcom's Vulcan

[Renato Golin via llvm-commits]

rengolin added inline comments.

================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:1806
@@ -1805,1 +1805,3 @@
     if (SecondOpcode == AArch64::Bcc) {
+      if (Subtarget.getProcFamily() == AArch64Subtarget::Vulcan) {
+        // All simple ALU operations that use one micro-op
----------------
pgode wrote:
> rengolin wrote:
> > rengolin wrote:
> > > This looks like a job for table-gen?
> > Also, please, don't add more getProcFamily calls, use sub-target features.
> For Cyclone, only subset of 6 instruction fusion cases are applicable, but for Vulcan, additional instructions are applicable, which may not be applicable for Cyclone. 
> So, to generalize this under one 'Subtarget Feature', getProcFamily call seems unavoidable. Please suggest.
> 
> Or, Is a new subtarget feature, such as 'FeatureMacroOpFusionVulcanSubset' a better option? 
> 
> Also, I am not sure about the table-gen option and how it can be done there. 
This is not a Cyclone vs Vulcan, but some things are more profitable than others, and it's possible, as you said, that this is the case for other cores, including Cyclone.

A new target feature is needed, but one with "vulcan" in its name, because that would defeat the purpose. We are getting rid of things like that, ex. "isLikeA9" because it's a trap.

The table-gen option would be to add a property to those instructions and use a check on that property here. I'll pass on the benefits of that apporach vs. this (other people can chime in), but this is orthogonal to the target feature discussion.

Bottom line is: we don't want *more* CPU checks.


http://reviews.llvm.org/D22042