[PATCH] D65497: [RISCV] Avoid generating AssertZext for RV64 when lowering floating Libcall

Shiva Chen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 6 02:31:57 PDT 2019


shiva0217 marked an inline comment as done.
shiva0217 added inline comments.


================
Comment at: include/llvm/CodeGen/TargetLowering.h:3033
+      bool doesNotReturn = false, bool isReturnValueUsed = true,
+      bool isPostTypeLegalization = false) const;
 
----------------
efriedma wrote:
> makeLibCall now takes five booleans, four of which are optional.  This is completely unreadable.
> 
> I can see a few ways to fix this. One, make the arguments non-optional, and use enum classes to name each one appropriately.  Two, introduce a makeLibCallOptions struct, with some appropriate defaults and methods to override each one, sort of like CallLoweringInfo.  Or three, introduce more entry points with appropriate names.
> 
> Also, I think you actually need to separately handle the return value and the argument; for example, float-to-int conversion has a non-extended argument, and a sign-extended result.  I don't think we generate calls to `__fixsfsi` at the moment on rv64; we call `__fixsfdi` instead.  But I'd rather fix the API to do the right thing, given we're changing it anyway.
Hi @efriedma,

I have created a parent patch D65795 to remove the optional arguments.
The patch introduces makeLibCallOptions struct which contains ArgListEntry and CallLoweringInfo classes. Each class has its appropriate constructor to initial default flags. We could remove optional arguments by passing makeLibCallOptions struct.
With this changing, argument flags could specify by ArgListEntry class and return value flag could specify by CallLoweringInfo class. 
Is the patch could meet what you thought in your mind? Thanks.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65497





More information about the llvm-commits mailing list