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

Akira Hatanaka ahatanak at gmail.com
Mon Mar 30 19:07:59 PDT 2015


Thank you for the review. A few comments inline.


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:804-805
@@ -800,1 +803,4 @@
 
+  bool hasCompatibleFnAttrs(const Function &Caller,
+                            const Function &Callee) const;
+
----------------
chandlerc wrote:
> 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.
Yes, I think we can look at the target independent attributes first.

================
Comment at: include/llvm/IR/EnumAttributes.td:3
@@ +2,3 @@
+class CompatCheckBase<bit V> {
+  bit Val;
+  let Val = V;
----------------
chandlerc wrote:
> 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?
I think we should use a list of attributes instead of a single bit if we want to check the compatibility between two different kinds of attributes. Do we need to do that for any of the target dependent or independent attributes we are currently using? I know we have to check attributes of the same kind in some cases (e.g., SanitizeAddress), but I'm not aware of any pair of different attribute kinds that can block inlining. 

Also, if we want to make the list of attributes short, we should probably use lists of incompatible attributes instead of lists of compatible ones (or define table-gen constructs for both), as I think most attributes are always compatible with each other regardless of their values.

http://reviews.llvm.org/D7802

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






More information about the llvm-commits mailing list