[PATCH] D129915: [InstCombine] Tighten up known library function signature tests (PR #56463)

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

I like the general approach, but haven't looked through the details.

> My preferred choice of a data structure would have been a constant array to represent the signatures with LibFunc as an index to retrieve each in constant time, but this would have required keeping the signatures sorted which feels like too much of a maintenance burden. With the (hopefully) intuitive notation for the signatures, the large switch statement seems like enough of an improvement to readability to avoid the common problems.

Just to throw it out there, one possible way to do this would be to include this information in TargetLibraryInfo.def. It already lists the signatures of all TLI functions, so it would make sense to also include them in machine-understandable form. For example, by adding a third macro:

  /// size_t strlen(const char *s);
  TLI_DEFINE_ENUM_INTERNAL(strlen)
  TLI_DEFINE_STRING_INTERNAL("strlen")
  TLI_DEFINE_SIG_INTERNAL({SizeT, Ptr})

The likely best solution would be to switch TargetLibraryInfo.def to use TableGen (like we do for intrinsics), which would allow generating this information with a compact encoding. That would take additional effort though.



================
Comment at: llvm/test/Transforms/InferFunctionAttrs/annotate-2.ll:6
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -inferattrs -opaque-pointers -S | FileCheck %s
+
----------------
msebor wrote:
> nikic wrote:
> > `-opaque-pointers` flag is not needed
> That's to make sure pointer type differences don't affect the test results.
Right, but just using `ptr` is sufficient for that, the `-opaque-pointers` flag isn't needed (it's enabled by default).


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

https://reviews.llvm.org/D129915