PR 23155: Change test to allow movzbl,movzwl in place of movb,movw instructions

[kbsmith1]

Summary

This change modifies a couple of unit tests to allow movzbl or movzwl instructions to be generated for
byte and word loads, as well as the existing instructions the test allowed (which were movb, movw).
This is first of several test updates to allow this generality/freedom in code generation for X86.
This is part of test suite changes needed to give compiler the freedom to choose either the zero extending
instructions or the plain byte/word instructions, since either are correct.

[spatel]

I'm concerned if we're adding variability where there was none before. I'm not sure if we have policy/guidelines on that, so adding some more experienced reviewers.

It would be nice to see how it's going to work at a higher level. Eg, will there be a new CPU attribute for partial register stalls that is used to trigger the different codegen? If so, can we replace the -mcpu for the test case and add a RUN line that has that attribute instead of using regex in these tests?

If this is related to PR23155, please note that here and in the commit message, so we'll have a paper trail for the motivation.

[kbsmith1]

Changed title, and explanation as Sanjay suggested. My expectation is that these tests should allow either the zero-extending
loads, or the plain byte/short instructions, as either are correct for the cpus specified.

[qcolombet]

Hi,

I agree with Sanjay that we shouldn’t add variability here.
Either we have a clear flag to choose one of the variants or we stick to just one.

Yes, both variants are correct here, but generating one or the other without being aware that another commit changes it sounds worrisome to me.

Cheers,
-Quentin

[kbsmith1]

There are a handful (6-8) tests that currently are checking for movw, where either movzwl or movw would be legal choices. There are quite a few more that specifically expect movb where movzbl would also be legal. My intent was to add a test that tested for the specific movb vs movzbl or movw vs movzwl when the transformations to change these opcodes got added.

My intent with this change-set was to get started changing tests where the test wasn't specifically trying to test ffor one or the other, where the test was simply testing for what the compiler currently did, rather than what it was legal to do. It seems like the principal of these unit tests should be to accept all legal outputs, except for the key place where they were checking for a specific piece of the implementation. That way the tests are not so fragile as heuristic changes are made. It seems wrong to have so many tests dependent on this choice, when that is really tangential to what they seem to be intending to check.

I was doing this to try to make these changes in small, easy to review pieces.

[kbsmith1]

Abandoned changes due to Quentin's opposition.

test/CodeGen/X86/half.ll

 ; RUN: llc < %s -march=x86-64 -mtriple=x86_64-unknown-linux-gnu -mcpu=corei7 -mattr=-f16c -asm-verbose=false \
 ; RUN:   | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-LIBCALL
 ; RUN: llc < %s -march=x86-64 -mtriple=x86_64-unknown-linux-gnu -mcpu=corei7 -mattr=+f16c -asm-verbose=false \
 ; RUN:    | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-F16C
 
 define void @test_load_store(half* %in, half* %out) {
 ; CHECK-LABEL: test_load_store:
-; CHECK: movw (%rdi), [[TMP:%[a-z0-9]+]]
-; CHECK: movw [[TMP]], (%rsi)
+; CHECK: {{movw|movzwl}} (%rdi), %{{e?}}[[TMP:[abcd]]]x
+; CHECK: movw %[[TMP]]x, (%rsi)
   %val = load half, half* %in
   store half %val, half* %out
   ret void
 }
 
 define i16 @test_bitcast_from_half(half* %addr) {
 ; CHECK-LABEL: test_bitcast_from_half:
 ; CHECK: movzwl (%rdi), %eax

test/CodeGen/X86/store-narrow.ll

entry:
   %D = or i32 %C, %B
   store i32 %D, i32* %a0, align 4
   ret void
 
 ; X64-LABEL: test1:
 ; X64: movb %sil, (%rdi)
 
 ; X32-LABEL: test1:
-; X32: movb 8(%esp), %al
+; X32: {{movb|movzbl}}  8(%esp), {{%al|%eax}}
 ; X32: movb %al, (%{{.*}})
 }
 
 define void @test2(i32* nocapture %a0, i8 zeroext %a1) nounwind ssp {
 entry:
   %A = load i32, i32* %a0, align 4
   %B = and i32 %A, -65281    ; 0xFFFF00FF
   %C = zext i8 %a1 to i32
   %CS = shl i32 %C, 8
   %D = or i32 %B, %CS
   store i32 %D, i32* %a0, align 4
   ret void
 ; X64-LABEL: test2:
 ; X64: movb %sil, 1(%rdi)
 
 ; X32-LABEL: test2:
-; X32: movb 8(%esp), %[[REG:[abcd]]]l
+; X32: {{movb|movzbl}}  8(%esp), %{{e?}}[[REG:[abcd]]]{{x|l}}
 ; X32: movb %[[REG]]l, 1(%{{.*}})
 }
 
 define void @test3(i32* nocapture %a0, i16 zeroext %a1) nounwind ssp {
 entry:
   %A = load i32, i32* %a0, align 4
   %B = and i32 %A, -65536    ; 0xFFFF0000
   %C = zext i16 %a1 to i32
   %D = or i32 %B, %C
   store i32 %D, i32* %a0, align 4
   ret void
 ; X64-LABEL: test3:
 ; X64: movw %si, (%rdi)
 
 ; X32-LABEL: test3:
-; X32: movw 8(%esp), %ax
+; X32: {{movw|movzwl}}  8(%esp), {{%ax|%eax}}
 ; X32: movw %ax, (%{{.*}})
 }
 
 define void @test4(i32* nocapture %a0, i16 zeroext %a1) nounwind ssp {
 entry:
   %A = load i32, i32* %a0, align 4
   %B = and i32 %A, 65535    ; 0x0000FFFF
   %C = zext i16 %a1 to i32
   %CS = shl i32 %C, 16
   %D = or i32 %B, %CS
   store i32 %D, i32* %a0, align 4
   ret void
 ; X64-LABEL: test4:
 ; X64: movw %si, 2(%rdi)
 
 ; X32-LABEL: test4:
-; X32: movw 8(%esp), %[[REG:[abcd]]]x
+; X32: {{movw|movzwl}}  8(%esp), %{{e?}}[[REG:[abcd]]]x
 ; X32: movw %[[REG]]x, 2(%{{.*}})
 }
 
 define void @test5(i64* nocapture %a0, i16 zeroext %a1) nounwind ssp {
 entry:
   %A = load i64, i64* %a0, align 4
   %B = and i64 %A, -4294901761    ; 0xFFFFFFFF0000FFFF
   %C = zext i16 %a1 to i64
   %CS = shl i64 %C, 16
   %D = or i64 %B, %CS
   store i64 %D, i64* %a0, align 4
   ret void
 ; X64-LABEL: test5:
 ; X64: movw %si, 2(%rdi)
 
 ; X32-LABEL: test5:
-; X32: movw 8(%esp), %[[REG:[abcd]]]x
+; X32: {{movw|movzwl}}  8(%esp), %{{e?}}[[REG:[abcd]]]x
 ; X32: movw %[[REG]]x, 2(%{{.*}})
 }
 
 define void @test6(i64* nocapture %a0, i8 zeroext %a1) nounwind ssp {
 entry:
   %A = load i64, i64* %a0, align 4
   %B = and i64 %A, -280375465082881    ; 0xFFFF00FFFFFFFFFF
   %C = zext i8 %a1 to i64
   %CS = shl i64 %C, 40
   %D = or i64 %B, %CS
   store i64 %D, i64* %a0, align 4
   ret void
 ; X64-LABEL: test6:
 ; X64: movb %sil, 5(%rdi)
 
 
 ; X32-LABEL: test6:
-; X32: movb 8(%esp), %[[REG:[abcd]l]]
-; X32: movb %[[REG]], 5(%{{.*}})
+; X32: {{movb|movzbl}}  8(%esp), %{{e?}}[[REG:[abcd]]]{{x|l}}
+; X32: movb %[[REG]]l, 5(%{{.*}})
 }
 
 define i32 @test7(i64* nocapture %a0, i8 zeroext %a1, i32* %P2) nounwind {
 entry:
   %OtherLoad = load i32 , i32 *%P2
   %A = load i64, i64* %a0, align 4
   %B = and i64 %A, -280375465082881    ; 0xFFFF00FFFFFFFFFF
   %C = zext i8 %a1 to i64
   %CS = shl i64 %C, 40
   %D = or i64 %B, %CS
   store i64 %D, i64* %a0, align 4
   ret i32 %OtherLoad
 ; X64-LABEL: test7:
 ; X64: movb %sil, 5(%rdi)
 
 
 ; X32-LABEL: test7:
-; X32: movb 8(%esp), %[[REG:[abcd]l]]
-; X32: movb %[[REG]], 5(%{{.*}})
+; X32: {{movb|movzbl}}  8(%esp), %{{e?}}[[REG:[abcd]]]{{x|l}}
+; X32: movb %[[REG]]l, 5(%{{.*}})
 }
 
 ; PR7833
 
 @g_16 = internal global i32 -1
 
 ; X64-LABEL: test8:
 ; X64-NEXT: movl _g_16(%rip), %eax