[PATCH] D69808: [RISCV GlobalISel] Add lowerReturn for calling conv.

[weiwei via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/RISCV/RISCVCallLowering.cpp:72
 
   if (Val != nullptr) {
+    // TODO: Only integer, pointer and aggregate types are supported now.
----------------
lewis-revill wrote:
> wwei wrote:
> > lewis-revill wrote:
> > > I don't know whether other people prefer this as well, but I'd prefer the condition here to be reversed and simply duplicate the 
> > > 
> > > ```
> > > MIRBuilder.insertInstr(Ret);
> > > return true;
> > > ```
> > > 
> > > IE:
> > > 
> > > ```
> > > if (Val == nullptr) {
> > >   MIRBuilder.insertInstr(Ret);
> > >   return true;
> > > }
> > > 
> > > // ...
> > > 
> > > MIRBuilder.insertInstr(Ret);
> > > return true;
> > > ```
> > To make the code look clearer and simpler, I will encapsulate a function to handle this condition.
> That's not quite what I meant by that, though I understand the desire to reduce code duplication. Personally I prefer a little bit of duplication of code like I suggested instead of adding another helper function.
In my opinion, using a helper function will make the code more clear and concise, which is really a personal preference issue, regardless of whether the implementation is correct or not. 


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

https://reviews.llvm.org/D69808