[PATCH] D40023: [RISCV] Implement ABI lowering

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/CodeGen/CGCall.cpp:1937
+        RetAttrs.addAttribute(llvm::Attribute::ZExt);
+    }
     // FALL THROUGH
----------------
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.


================
Comment at: lib/CodeGen/TargetInfo.cpp:8790
+private:
+  unsigned XLen;
+  static const int NumArgGPRs = 8;
----------------
Is "XLen" a term that anyone working with RISCV would recognize?  Because even if it is, it feels like something you should document here — just "standard pointer size in bits" would be fine.


================
Comment at: lib/CodeGen/TargetInfo.cpp:8858
+
+  uint64_t Size = getContext().getTypeSize(Ty);
+  uint64_t NeededAlign = getContext().getTypeAlign(Ty);
----------------
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.


https://reviews.llvm.org/D40023