rjmccall added inline comments.

================
Comment at: lib/CodeGen/CGCall.cpp:1937
+        RetAttrs.addAttribute(llvm::Attribute::ZExt);
+    }
     // FALL THROUGH
----------------
asb wrote:
> rjmccall wrote:
> > asb wrote:
> > > rjmccall wrote:
> > > > I feel like a better design would be to record whether to do a sext or 
> > > > a zext in the ABIArgInfo.  Add getSignExtend and getZeroExtend static 
> > > > functions to ABIArgInfo and make getExtend a convenience function that 
> > > > takes a QualType and uses its signedness.
> > > I could see how that might be cleaner, but that's a larger refactoring 
> > > that's going to touch a lot more code. Are you happy for this patch to 
> > > stick with this more incremental change (applying the same sign-extension 
> > > logic to return values as is used for arguments), and to leave your 
> > > suggested refactoring for a future patch?
> > I won't insist that you do it, but I don't think this refactor would be as 
> > bad as you think.  Doing these refactors incrementally when we realize that 
> > the existing infrastructure is failing us in some way is how we make sure 
> > they actually happen.  Individual contributors rarely have any incentive to 
> > ever do that "future patch".
> I've submitted this refactoring in D41999.
Much appreciated, thanks!


https://reviews.llvm.org/D40023



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to