[PATCH] D12581: [MergeFuncs] Fix callsite attributes in thunk generation

[JF Bastien via llvm-commits]

jfb added inline comments.

================
Comment at: lib/Transforms/IPO/MergeFunctions.cpp:1558
@@ +1557,3 @@
+      // etc. No code actually looks up attributes from the called function,
+      // because then it would fail on indirect calls.
+
----------------
jrkoenig wrote:
> jfb wrote:
> > Why would non-null be missing? Could you add a test that shows this?
> > 
> > I'm not sure I get why the bitcast is needed here, and why you can't just drop the comment. It looks like it's there to match parameters/returns that are layout compatible but have different names. Why not cast those, instead of the function signature?
> There can be differences between attributes at a callsite and on a function, especially when a call was cross-TU but which is now a static callsite due to linking. The ABI attributes from a callsite and function must match (else the result is UB), and this must already have been true of the input because Old and New have the same ABI attributes. No ABI attributes are transferred in the test suite or in any of my LTO builds, but other attributes are. If transferring other attributes is valuable as an optimization then it should happen in it's own optimization pass, not here.
Ah OK, that sounds unfortunate but not worth addressing in the patch. Could you clarify the comment with this?


http://reviews.llvm.org/D12581