[PATCH] D103992: [InstCombine / BuildLibCalls] Add parameter attributes from the prototype.

Thu Jun 24 11:41:21 PDT 2021

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm, go ahead and remove the TODO before landing

================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1120-1121
+  B.CreateMemCpy(Dst, Align(1), CI->getArgOperand(1), Align(1), N);
   // Propagate attributes, but memcpy has no return value, so make sure that
   // any return attributes are compliant.
   // TODO: Attach return value attributes to the 1st operand to preserve them?
----------------
jonpa wrote:
> rnk wrote:
> > This change makes this comment stale. I checked using blame, and this setAttributes call was added in rGe80fcf03407acd6429d07e4a45185ac546ffa37c by @xbolva00 to propagate the `nonnull` annotations. However, I don't see any changes to the tests that were added, so maybe we don't need this any longer. Maybe the attributes are present on the memcpy declaration already.
> > 
> > In conclusion, I think the change looks good, just remove this stale comment. @xbolva00 , feel free to check that logic.
> Removed (not sure about leaving the 'TODO'..?)
> 
The TODO was added in D95088, which you have removed the need for. I believe it can also be removed.

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

https://reviews.llvm.org/D103992