[PATCH] Add logical ops to Mips fast-isel
Daniel Sanders
daniel.sanders at imgtec.com
Mon Dec 15 02:26:37 PST 2014
LGTM with whitespace nits and an improved test case. Currently many tests are missing and i32_test lacks any CHECK directives. There's a few more comments on this subject below.
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:240
@@ +239,3 @@
+ } else
+ llvm_unreachable("unexpected opcode");
+ unsigned LHSReg = getRegForValue(LHS);
----------------
Formatting nit: needs a blank line after this
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:244
@@ +243,3 @@
+ if (!ResultReg)
+ return 0;
+ unsigned RHSReg;
----------------
Formatting nit: needs a blank line after this
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:247
@@ +246,3 @@
+ if (!LHSReg)
+ return 0;
+ if (const auto *C = dyn_cast<ConstantInt>(RHS))
----------------
Formatting nit: needs a blank line after this
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:251
@@ +250,3 @@
+ else
+ RHSReg = getRegForValue(RHS);
+ if (!RHSReg)
----------------
Formatting nit: needs a blank line after this
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:253
@@ +252,3 @@
+ if (!RHSReg)
+ return 0;
+ emitInst(Opc, ResultReg).addReg(LHSReg).addReg(RHSReg);
----------------
Formatting nit: needs a blank line after this
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:485
@@ -435,1 +484,3 @@
+ return false;
+}
bool MipsFastISel::isLoadTypeLegal(Type *Ty, MVT &VT) {
----------------
Formatting nit: needs a blank line after this
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:741
@@ +740,3 @@
+ if (!isTypeSupported(I->getType(), VT))
+ return false;
+ unsigned ResultReg;
----------------
Formatting nit: needs a blank line after this
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:755
@@ +754,3 @@
+ break;
+ }
+ if (!ResultReg)
----------------
Formatting nit: needs a blank line after this
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:757
@@ +756,3 @@
+ if (!ResultReg)
+ return false;
+ updateValueMap(I, ResultReg);
----------------
Formatting nit: needs a blank line after this
================
Comment at: test/CodeGen/Mips/Fast-ISel/logopm.ll:6-9
@@ +5,6 @@
+
+; ModuleID = 'logop.c'
+target datalayout = "e-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64"
+target triple = "mipsel--linux-gnu"
+
+ at i1_1 = global i8 1, align 1
----------------
We don't need these lines
================
Comment at: test/CodeGen/Mips/Fast-ISel/logopm.ll:31-111
@@ +30,83 @@
+
+; Function Attrs: nounwind
+define void @i1_test() #0 {
+entry:
+; CHECK-LABEL: .ent i1_test
+ %0 = load i8* @i1_1, align 1
+ %conv = trunc i8 %0 to i1
+ %and = and i1 1, %conv
+; CHECK: and ${{[0-9]+}}, ${{[0-9]+}}, ${{[0-9]+}}
+ %conv1 = zext i1 %and to i8
+ store i8 %conv1, i8* @j1_1, align 1
+ %1 = load i8* @i1_1, align 1
+ %conv2 = trunc i8 %1 to i1
+ %2 = load i8* @i1_2, align 1
+ %conv3 = trunc i8 %2 to i1
+ %and4 = and i1 %conv2, %conv3
+; CHECK: and ${{[0-9]+}}, ${{[0-9]+}}, ${{[0-9]+}}
+ %conv5 = zext i1 %and4 to i8
+ store i8 %conv5, i8* @j1_2, align 1
+ %3 = load i8* @i1_2, align 1
+ %conv6 = trunc i8 %3 to i1
+ %4 = load i8* @i1_3, align 1
+ %conv7 = trunc i8 %4 to i1
+ %and8 = and i1 %conv6, %conv7
+; CHECK: and ${{[0-9]+}}, ${{[0-9]+}}, ${{[0-9]+}}
+ %conv9 = zext i1 %and8 to i8
+ store i8 %conv9, i8* @j1_3, align 1
+ %5 = load i8* @i1_3, align 1
+ %conv10 = trunc i8 %5 to i1
+ %6 = load i8* @i1_4, align 1
+ %conv11 = trunc i8 %6 to i1
+ %and12 = and i1 %conv10, %conv11
+; CHECK: and ${{[0-9]+}}, ${{[0-9]+}}, ${{[0-9]+}}
+ %conv13 = zext i1 %and12 to i8
+ store i8 %conv13, i8* @j1_4, align 1
+ ret void
+}
+
+; Function Attrs: nounwind
+define void @i8_test() #0 {
+entry:
+; CHECK-LABEL: .ent i8_test
+ %0 = load i8* @i8_1, align 1
+ %or = or i8 %0, 225
+ store i8 %or, i8* @j8_1, align 1
+; CHECK: or ${{[0-9]+}}, ${{[0-9]+}}, ${{[0-9]+}}
+ %1 = load i8* @i8_1, align 1
+ %2 = load i8* @i8_2, align 1
+ %or4 = or i8 %1, %2
+; CHECK: or ${{[0-9]+}}, ${{[0-9]+}}, ${{[0-9]+}}
+ store i8 %or4, i8* @j8_2, align 1
+ ret void
+}
+
+; Function Attrs: nounwind
+define void @i16_test() #0 {
+entry:
+; CHECK-LABEL: .ent i16_test
+ %0 = load i16* @i16_1, align 2
+ %xor = xor i16 241, %0
+ store i16 %xor, i16* @j16_1, align 2
+; CHECK: xor ${{[0-9]+}}, ${{[0-9]+}}, ${{[0-9]+}}
+ %1 = load i16* @i16_1, align 2
+ %2 = load i16* @i16_2, align 2
+ %xor4 = xor i16 %1, %2
+; CHECK: xor ${{[0-9]+}}, ${{[0-9]+}}, ${{[0-9]+}}
+ store i16 %xor4, i16* @j16_2, align 2
+ ret void
+}
+
+; Function Attrs: nounwind
+define void @i32_test() #0 {
+entry:
+ %0 = load i32* @i32_1, align 4
+ %xor = xor i32 %0, -16908096
+ store i32 %xor, i32* @j32_1, align 4
+ %1 = load i32* @i32_1, align 4
+ %2 = load i32* @i32_2, align 4
+ %xor1 = xor i32 %1, %2
+ store i32 %xor1, i32* @j32_2, align 4
+ ret void
+}
+
----------------
The tests are incomplete. Also, i32_test doesn't have any CHECK's.
The missing tests are:
* i1 xor/or
* i1 and/xor/or using immediates
* i8 and/xor
* i8 and/xor/or using immediates
* i16 and/or
* i16 and/xor/or using immediates
* i32 and/xor/or (there's code for xor but no checks)
* i32 and/xor/or using immediates (there's code for xor but no checks)
Also, please do one test per function (i.e. one function for the register form, and one for the immediate form). It makes it easier to tell what is covered.
Finally, test_i1 doesn't need to be significantly different from the others. Taking test_i8 and switching all the types to i1 (and switching to the correct global) will give you a simple test for 1-bit integers.
================
Comment at: test/CodeGen/Mips/Fast-ISel/logopm.ll:112-210
@@ +111,100 @@
+}
+
+; Function Attrs: nounwind
+define i32 @main() #0 {
+entry:
+ %retval = alloca i32, align 4
+ store i32 0, i32* %retval
+ call void @i1_test()
+ %0 = load i8* @j1_1, align 1
+ %conv = sext i8 %0 to i32
+ %cmp = icmp ne i32 %conv, 1
+ br i1 %cmp, label %if.then, label %lor.lhs.false
+
+lor.lhs.false: ; preds = %entry
+ %1 = load i8* @j1_2, align 1
+ %conv2 = sext i8 %1 to i32
+ %cmp3 = icmp ne i32 %conv2, 0
+ br i1 %cmp3, label %if.then, label %lor.lhs.false5
+
+lor.lhs.false5: ; preds = %lor.lhs.false
+ %2 = load i8* @j1_3, align 1
+ %conv6 = sext i8 %2 to i32
+ %cmp7 = icmp ne i32 %conv6, 0
+ br i1 %cmp7, label %if.then, label %lor.lhs.false9
+
+lor.lhs.false9: ; preds = %lor.lhs.false5
+ %3 = load i8* @j1_4, align 1
+ %conv10 = sext i8 %3 to i32
+ %cmp11 = icmp ne i32 %conv10, 1
+ br i1 %cmp11, label %if.then, label %if.end
+
+if.then: ; preds = %lor.lhs.false9, %lor.lhs.false5, %lor.lhs.false, %entry
+ call void @abort() #2
+ unreachable
+
+if.end: ; preds = %lor.lhs.false9
+ call void @i8_test()
+ %4 = load i8* @j8_1, align 1
+ %conv13 = zext i8 %4 to i32
+ %cmp14 = icmp ne i32 %conv13, 229
+ br i1 %cmp14, label %if.then20, label %lor.lhs.false16
+
+lor.lhs.false16: ; preds = %if.end
+ %5 = load i8* @j8_2, align 1
+ %conv17 = zext i8 %5 to i32
+ %cmp18 = icmp ne i32 %conv17, 199
+ br i1 %cmp18, label %if.then20, label %if.end21
+
+if.then20: ; preds = %lor.lhs.false16, %if.end
+ call void @abort() #2
+ unreachable
+
+if.end21: ; preds = %lor.lhs.false16
+ call void @i16_test()
+ %6 = load i16* @j16_1, align 2
+ %conv22 = zext i16 %6 to i32
+ %cmp23 = icmp ne i32 %conv22, 57395
+ br i1 %cmp23, label %if.then29, label %lor.lhs.false25
+
+lor.lhs.false25: ; preds = %if.end21
+ %7 = load i16* @j16_2, align 2
+ %conv26 = zext i16 %7 to i32
+ %cmp27 = icmp ne i32 %conv26, 4131
+ br i1 %cmp27, label %if.then29, label %if.end30
+
+if.then29: ; preds = %lor.lhs.false25, %if.end21
+ call void @abort() #2
+ unreachable
+
+if.end30: ; preds = %lor.lhs.false25
+ call void @i32_test()
+ %8 = load i32* @j32_1, align 4
+ %cmp31 = icmp ne i32 %8, -17755515
+ br i1 %cmp31, label %if.then36, label %lor.lhs.false33
+
+lor.lhs.false33: ; preds = %if.end30
+ %9 = load i32* @j32_2, align 4
+ %cmp34 = icmp ne i32 %9, 16712532
+ br i1 %cmp34, label %if.then36, label %if.end37
+
+if.then36: ; preds = %lor.lhs.false33, %if.end30
+ call void @abort() #2
+ unreachable
+
+if.end37: ; preds = %lor.lhs.false33
+ ret i32 0
+}
+
+; Function Attrs: noreturn
+declare void @abort() #1
+
+attributes #0 = { nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { noreturn "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #2 = { noreturn }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+!0 = metadata !{i32 1, metadata !"PIC Level", i32 2}
+!1 = metadata !{metadata !"clang version 3.6.0 (gitosis at dmz-portal.mips.com:clang.git 59d05423ef08bc76eeedb88bb7bfa0b0e0415711) (gitosis at dmz-portal.mips.com:llvm.git 431113b8f2ea22b437e86d06a0ce5fbe78e2d4cb)"}
----------------
We don't need these lines. test/CodeGen tests are not executable.
http://reviews.llvm.org/D6599
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list