[PATCH] D87301: [IRSim][IROutliner] Adding consistent function attribute merging

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/IR/Attributes.cpp:2077
+  setOR<UseSampleProfileAttr>(Base, ToMerge);
+}
----------------
AndrewLitteken wrote:
> jdoerfert wrote:
> > AndrewLitteken wrote:
> > > jdoerfert wrote:
> > > > Are we sure we want/need to duplicate this here in addition to the merge rules at the end of `llvm/include/llvm/IR/Attributes.td` ?
> > > We can probably just call `mergeFnAttrs` from this function and merge the attributes from SanitizeAddress separately.
> > Hm, why are the "SanitizeAddress" address ones not handled in `mergeFnAttrs` as well? Doesn't the inliner need to know about them too? I would prefer any way we avoid a second list, that is doomed to cause problems.
> I think because it uses the `areInlineCompatible`, like above, where I'm merging them for combining two outlined functions functions together and make sure that the information isn't lost.
> 
> That said, I'm not super familiar with these attributes, so this may not be considered safe.
I don't get it. Why can't we call `mergeFnAttrs(Caller, Callee);` here?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87301/new/

https://reviews.llvm.org/D87301