Thanks. I think this patch is heading in the right direction. I have a 
lingering doubt about how ABIArgInfo::Extend is used in a number of places but 
we're only changing one of them. I looked through the others and didn't see 
anything that needed changing though. @atanasyan, do you mind taking a look too?

One other thing I'm thinking about is that we ought to keep the code to handle 
this in the same place as the rest (in MipsABIInfo::classifyArgumentType()). 
This would require adding an ABIArgInfo::SignExtend.

Also, have you tried a variant of your failing test but using varargs? This 
uses a different code path so we ought to check that too.


================
Comment at: lib/CodeGen/TargetInfo.cpp:5797-5801
@@ +5796,7 @@
+bool MipsABIInfo::shouldSignExtUnsignedType(QualType Ty) const {
+  int TySize = getContext().getTypeSize(Ty);
+  if(Ty->isUnsignedIntegerOrEnumerationType() && TySize == 32)
+    return true;
+  else
+    return false;
+}
----------------
Nit: Please add a comment explaining why unsigned i32 should be sign-extended. 
It won't be obvious to people who don't already know about this quirk of the 
ABI.

http://reviews.llvm.org/D9198

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to