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

Daniel Sanders daniel.sanders at imgtec.com
Wed Feb 25 08:15:11 PST 2015


Well spotted. I thought I had caught all of these bugs. Sadly it's just missed LLVM 3.6.0.

Could you move the tests into test/CodeGen/Mips/cconv/ and name them arguments-varargs-*.ll? I'm trying to keep all calling convention tests together. With the tests moved and the nits fixed, it will LGTM.
Do you need someone to commit it?

Ideally, they would be of roughly the same form as test/CodeGen/Mips/cconv/arguments-varargs.ll and they'd check the assembly for big/little-endian O32, N32, and N64 like that test does.  This bit can be done as a follow-up if need be.


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()",
----------------
I'm not sure the i64 is necessary but it's not doing any harm either so no change required.

================
Comment at: test/CodeGen/Mips/small-structs-byte.ll:1
@@ +1,2 @@
+; RUN: llc --march=mips64 -mcpu=mips64r2 < %s | FileCheck %s
+
----------------
Nit: Are these tests generated from C source? If so, could you add the source file as a comment?

The other calling convention tests are (mostly) handwritten LLVM-IR but that doesn't really work for varargs so I've been making an exception for them.

================
Comment at: test/CodeGen/Mips/small-structs-byte.ll:15
@@ +14,3 @@
+
+; Function Attrs: nounwind
+declare void @varArgF_SmallStruct(i8* %c, ...) 
----------------
Nit: This comment isn't useful. Likewise for the other examples in the tests.

================
Comment at: test/CodeGen/Mips/small-structs-combinations.ll:14
@@ +13,3 @@
+
+
+; Function Attrs: nounwind
----------------
Nit: Duplicate blank line

================
Comment at: test/CodeGen/Mips/small-structs-multiple-args.ll:79
@@ +78,3 @@
+ ; CHECK: dsll $[[R1:[0-9]+]], $[[R2:[0-9]+]], 56
+
+}
----------------
Nit: Blank line

http://reviews.llvm.org/D7881

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






More information about the llvm-commits mailing list