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

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa updated this revision to Diff 481956.
jonpa added a comment.

Getting back to this now finally, sorry for the delay.

> It's probably more clear to pass in the attribute list like GCOVProfiling does...

I added a new method to TLI "getAttrList()" to quickly insert an AttributeList e.g. like

  WarningFn = M.getOrInsertFunction("__msan_warning",
                                     TLI.getAttrList(C, {0}, false),
                                     IRB.getVoidTy(), IRB.getInt32Ty());

I have now gone through AddressSanitizer.cpp, MemorySanitizer.cpp, ThreadSanitizer.cpp, OMPKinds.def and GCOVProfiling.cpp, which all had many extension attributes missing.

Don't know much about the signedness of the different args, which of course is important to get correct. I guess I have used zero-extension mostly - it would be good to get this reviewed properly. In OMPKinds.def I added ZExt to all narrow int args, except SExt for args marked with /* Int */.

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

It's slightly confusing that these attributes are sometimes added unconditionally (as target is free to ignore them), but yet we sometimes use getExtAttrForI32Param() (directly or indirectly) to only add as requested. I have assumed that since there exists these target hooks guarding this, they should be used as much as possible and feasible. In OMPKinds.def for instance they are however added unconditionally.

Does this approach look ok? If so, I suppose I should continue with adding tests for the emitted attributes - e.g. AddressSanitizer tests seems to be run only for X86 which still do not (should not per above) get the attributes emitted...


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

https://reviews.llvm.org/D133949

Files:
  clang/test/OpenMP/barrier_codegen.cpp
  llvm/include/llvm/Analysis/TargetLibraryInfo.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
  llvm/test/Instrumentation/MemorySanitizer/SystemZ/vararg.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Transforms/OpenMP/add_attributes.ll
  llvm/test/Transforms/OpenMP/hide_mem_transfer_latency.ll
  llvm/test/Transforms/OpenMP/parallel_level_fold.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133949.481956.patch
Type: text/x-patch
Size: 139594 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221212/0abd4b98/attachment.bin>