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

[Martin Sebor via Phabricator via llvm-commits]

msebor marked 3 inline comments as done.
msebor added a comment.

Thanks for the comments.  I actually wasn't quite sure the patch was ready for review because there are more functions where the checking should be tightened up. But since you've already gone through it let me ask a few questions:

1. Should I submit one patch with all of them or two or more smaller patches?
2. The functions are not in any particular order, which tends to lead to repetitive checks in different places, and is also more error-prone.  I've already moved some of them together (grouped by the header they're declared in) but I wonder: should I do it for all of them? (I.e., have groups by header with no repetitive checks in each group.)
3. The argument checking is also exceedingly verbose with the differences often being just the argument number or type.  These differences would be easier to see (and harder to miss when adding new checks) if we cut down on the amount of superfluous text (e.g., avoided calling `getParamType(N)` for each argument and instead provided an accessor, like `operator[]`).  Would you support this change?



================
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
+
----------------
nikic wrote:
> `-opaque-pointers` flag is not needed
That's to make sure pointer type differences don't affect the test results.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129915