[PATCH] D133949: Make sure the right parameter extension attributes are added in various instrumentation passes.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> MemorySanitizer:createUserspaceApi(): Some arg ext attributes here are now added with getAttrList() instead, which means they are only added if the target requires it. Tests for this were therefore moved to SystemZ/vararg.ll

This is right.  We shouldn't be adding attributes unconditionally; at best, it's noise on targets where the attributes aren't significant.



================
Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:400
+  // the same signedness.
+  AttributeList getAttrList(LLVMContext *C, ArrayRef<unsigned> ArgNos,
+                            bool Signed, bool Ret = false) const {
----------------
This looks reasonable.


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2564
   AMDGPUAddressPrivate = M.getOrInsertFunction(
-      kAMDGPUAddressPrivateName, IRB.getInt1Ty(), IRB.getInt8PtrTy());
+      kAMDGPUAddressPrivateName, TLI->getAttrList(C, {}, false, true/*Ret*/),
+      IRB.getInt1Ty(), IRB.getInt8PtrTy());
----------------
Usually we prefer to put the comment before the value.  See https://llvm.org/docs/CodingStandards.html#comment-formatting 

Probably also worth putting comments for the "signed" boolean.


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

https://reviews.llvm.org/D133949