rjmccall added inline comments.
================
Comment at: lib/CodeGen/CGCall.cpp:1937
+ RetAttrs.addAttribute(llvm::Attribute::ZExt);
+ }
// FALL THROUGH
----------------
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".
================
Comment at: lib/CodeGen/TargetInfo.cpp:8858
+
+ uint64_t Size = getContext().getTypeSize(Ty);
+ uint64_t NeededAlign = getContext().getTypeAlign(Ty);
----------------
asb wrote:
> rjmccall wrote:
> > I would encourage you to use CharUnits and getTypeSizeInChars more
> > consistently in this code; it would simplify some of the conversions you're
> > doing and eliminate some of the risk of forgetting a bits-to-bytes
> > conversion. It looks like storing XLen as a CharUnits would also make this
> > easier.
> XLen is defined throughout the RISC-V ISA and ABI documentation as the width
> of the integer ('x') registers in bits. I've modified EmitVAArg to make use
> of CharUnits to avoid a conversion - there don't seem to be any other
> bit/byte conversions I can see.
Okay, I can accept that, 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
