[PATCH] D21739: [TLI] Add functions determining if int parameters/returns should be zeroext/signext.

Hal Finkel via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 10 08:54:55 PST 2016


hfinkel added inline comments.


================
Comment at: include/llvm/Analysis/TargetLibraryInfo.h:308
+  /// signext attribute if they correspond to C-level int or unsigned int.
+  bool shouldSignExtI32Param() const {
+    return Impl->ShouldSignExtI32Param;
----------------
I don't like this interface. Looking at D21736, for signed parameters, you need to call two query functions. Can you please combine these into one function like this:

  bool shouldExtI32Param(bool Signed = true) const { ... }

Another option is to make two functions, like:

  bool shouldExtSignedI32Param() const { ... }
  bool shouldExtUnsignedI32Param() const { ... }

I don't have a strong preference.



================
Comment at: lib/Analysis/TargetLibraryInfo.cpp:428
+  // Mips, on the other hand, needs signext on i32 parameters corresponding
+  // to both signed and unsigned ints.
+  if (T.getArch() == Triple::mips || T.getArch() == Triple::mipsel ||
----------------
To be clear, they sign extend even unsigned numbers?


Repository:
  rL LLVM

https://reviews.llvm.org/D21739





More information about the llvm-commits mailing list