[PATCH] Fix justify error for small structures in variable arguments for MIPS64 big endian

Daniel Sanders daniel.sanders at imgtec.com
Thu Feb 26 08:20:38 PST 2015


In http://reviews.llvm.org/D7881#130408, @abeserminji wrote:

> In http://reviews.llvm.org/D7881#130264, @dsanders wrote:
>
> > Bugzilla was playing up yesterday but I've been able to look at PR21608 again and there's something I should mention here. I believe this patch delivers the correct and intended behaviour but it should be noted that GCC miscompiles this case in the same way. I mention this because our criteria for all the other calling convention bugs was "matches GCC's behaviour" since it implements the de-facto calling convention for Mips which appears to differ slightly from the documented calling convention. On this occasion, it makes sense for us to differ from GCC's current (broken) behaviour and have GCC fixed later.
>
>
> I think that the example provided in PR21608 is not correct. 
>  The reason why I think so is because integers are passed as variable arguments and structures are expected. 
>  I would say that function //test_i32(...)// should be called like this:
>
>     printf("%d\n", test_i32(
>   				"", 
>   				(struct f){-1},
>   				(struct f){1}, 
>   				(struct f){2}, 
>   				(struct f){3}, 
>   				(struct f){4}, 
>   				(struct f){5}, 
>   				(struct f){6},
>   				(struct f){7}, 	
>   				(struct f){8}, 
>   				(struct f){9}, 	
>   				(struct f){10}
>   			)
>   	);
>
>
> Now structures are passed instead of integers.
>  And if you compile this example with GCC it works good.


Now that you've mentioned it I see it too. I agree the test is broken. There should be a gcc bugzilla ticket with the same test case somewhere. I'll see if I can track it down and mark it invalid.


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/Mips/MipsCallingConv.td:162
@@ -161,1 +161,3 @@
 def CC_MipsN_VarArg : CallingConv<[
+  CCIfType<[i8, i16, i32, i64],
+      CCIfSubtargetNot<"isLittle()",
----------------
abeserminji wrote:
> dsanders wrote:
> > I'm not sure the i64 is necessary but it's not doing any harm either so no change required.
> The i64 is used here for structures bigger than 32 bits.
> Without it, structures bigger than 32 bits are not shifted as they should be.
Good point. Sorry for the noise.

http://reviews.llvm.org/D7881

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






More information about the llvm-commits mailing list