[PATCH] [Inliner] Use whitelist instead of blacklist when checking function attribute compatibility and make the check stricter

[Chandler Carruth]

Ok, finally back to this. Generally I do like the direction. Some more specific guidelines below.


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:804-805
@@ -800,1 +803,4 @@
 
+  bool hasCompatibleFnAttrs(const Function &Caller,
+                            const Function &Callee) const;
+
----------------
I would start off without trying to solve the problem for textual target attributes, and just take a conservative approach for them until we have a good use case, and can layer it on top.

================
Comment at: include/llvm/IR/EnumAttributes.td:1
@@ +1,2 @@
+
+class CompatCheckBase<bit V> {
----------------
I really like this general approach. Could you start off the .td file with a comment explaining the syntax that should be used?

================
Comment at: include/llvm/IR/EnumAttributes.td:3
@@ +2,3 @@
+class CompatCheckBase<bit V> {
+  bit Val;
+  let Val = V;
----------------
Rather than a single bit for compatibility checking, I'd like this to be a list of attributes to fall back on when merging.

I'm thinking of a structure like this.

  def X : Attr<..., CompatSequence<[A, B, C]>>;
  def Y : Attr<..., CompatSequence<[B, C]>>;

This would say that X is compatible with A, B, or C. So if we're inlining a Callee with X into A, B, or C, its fine. It would also mean that if we are inlining a callee with X into a caller Y, we could switch to attribute B instead as the first common attribute between them.

Thoughts?

http://reviews.llvm.org/D7802

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/