[PATCH] Add logical ops to Mips fast-isel

Daniel Sanders daniel.sanders at imgtec.com
Mon Mar 2 07:19:57 PST 2015


================
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
+}
+
----------------
dsanders wrote:
> rkotler wrote:
> > rkotler wrote:
> > > dsanders wrote:
> > > > 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.
> > > I like to make the putback tests be executable and possibly removing the "main function" before putting them back so I'm sure that they work.
> > I32 is handled without this patch already by means of the tablegen patterns.
> > 
> > The need for this patch is to handle things that normally would be taken care of by the legalizer. For example: xor or any non 32 integer types.
> > I like to make the putback tests be executable and possibly removing the "main function" before putting them back so I'm sure that they work.
> 
> I agree that checking that we can execute real C/C++ code is a good thing to have in addition to the test/CodeGen tests. However, the test/CodeGen tests are unit tests for LLVM-IR to assembly. They are not intended to be executable or language-specific (although tests for specific bugs often will be) and redundant code in these tests just costs everyone additional test overhead without adding any value.
> 
> There are people working on adding executable test support though. I'm not sure what the current state of it is but the last I saw on the subject was '[LLVMdev] QEMU testing for LIT execution tests' in September. It's aimed at compiler-rt/libcxx/etc. testing but there's no reason trivial source->execution tests couldn't use the same mechanism. Maybe we could end up with a set of tests somewhere like test/CompileAndRun/C99/... or something along those lines.
> 
> > I32 is handled without this patch already by means of the tablegen patterns.
> 
> We should still check the output assembly so that we know that the tablegen patterns do the right thing in Fast ISel. At the moment you have the test but don't check that it passes.
>> I32 is handled without this patch already by means of the tablegen patterns.
> We should still check the output assembly so that we know that the tablegen patterns do the right thing in Fast ISel. At the moment you have the test but don't check that it passes.

Please disregard this comment. It appears I've commented on the first revision and not the latest.

================
Comment at: test/CodeGen/Mips/Fast-ISel/logopm.ll:37-38
@@ +36,4 @@
+  %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
----------------
dsanders wrote:
> Something I missed on the previous read throughs: None the instructions with the immediate check that the immediate is there.
> 
> I understand that we don't emit 'andi $1, $2, 3' and similar yet but we should check that the immediate is materialized in a register that is used by the instruction that would have had it.
> Something I missed on the previous read throughs: None the instructions with the immediate check that the immediate is there.
> 
> I understand that we don't emit 'andi $1, $2, 3' and similar yet but we should check that the immediate is materialized in a register that is used by the instruction that would have had it.

Please disregard this comment. I've commented on the first revision and not the latest.

http://reviews.llvm.org/D6599

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






More information about the llvm-commits mailing list