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

Tue Sep 15 22:05:38 PDT 2020

AndrewLitteken added inline comments.

================
Comment at: llvm/lib/IR/Attributes.cpp:2077
+  setOR<UseSampleProfileAttr>(Base, ToMerge);
+}
----------------
jdoerfert wrote:
> 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?
It should be, I think I uploaded the wrong diff. Sorry about that.

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

https://reviews.llvm.org/D87301