[PATCH] D40696: Enable aggressive FMA on T99 and provide AArch64 option for other micro-arch's

Wed Jan 24 01:39:00 PST 2018

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

LGTM with 2 small comments, I think all comments should be addressed now, thanks!  Please wait with committing for another day or so, to give people some time to raise additional concerns.

When committing, please adjust the commit title & message to mention the target feature rather than the option. And it would be great if you could prefix the title again with [AArch64]

================
Comment at: lib/Target/AArch64/AArch64.td:153
+def FeatureAggressiveFMA :
+  SubtargetFeature<"enable-aggressive-fma",
+                   "HasAggressiveFMA",
----------------
Just `aggressive-fma` seems more in line with the existing target feature names. `+aggressive-fma` enables the target feature, `-aggressive-fma` explicitly disables it.

================
Comment at: lib/Target/AArch64/AArch64.td:409
+                                          HasV8_1aOps,
+                                          FeatureAggressiveFMA]>;

----------------
nit: move it to the other FeatureXXX in the list.

Repository:
  rL LLVM

https://reviews.llvm.org/D40696