Re: [PATCH v7 15/15] qla2xxx: Fix endianness annotations in source files

[Finn Thain <fthain@telegraphics.com.au>]

I went through commit 7ffa5b939751 ("scsi: qla2xxx: Fix endianness 
annotations in source files") and produced a new version without the 
'const' changes and without the new local variables. I also tweaked a few 
line breaks so that the line numbers did not change.

I then built the driver both with and without the patch:

mkdir /tmp/patched /tmp/unpatched
rm drivers/scsi/qla2xxx/built-in.a drivers/scsi/qla2xxx/*.o *.[is] 
make KBUILD_CFLAGS=-save-temps drivers/scsi/qla2xxx/built-in.a
mv *.s /tmp/patched/
git checkout @^
rm drivers/scsi/qla2xxx/built-in.a drivers/scsi/qla2xxx/*.o *.[is] 
make KBUILD_CFLAGS=-save-temps drivers/scsi/qla2xxx/built-in.a
mv *.s /tmp/unpatched/
meld /tmp/patched/ /tmp/unpatched/

This revealed some differences, in both x86_64 and i686 builds.

1. The wordsize changes in qla24xx_els_ct_entry() apparently prevented the 
compiler from emitting zero-extension mov (movzwl) instructions.

diff -ru /tmp/unpatched/qla_isr.s /tmp/patched/qla_isr.s
--- /tmp/unpatched/qla_isr.s    2020-05-24 13:59:04.327714528 +1000
+++ /tmp/patched/qla_isr.s      2020-05-24 13:56:58.437529886 +1000
@@ -8947,11 +8947,9 @@
        movw    %ax, -26(%rbp)
        movq    -104(%rbp), %rax
        movl    36(%rax), %eax
-       movzwl  %ax, %eax
        movl    %eax, -76(%rbp)
        movq    -104(%rbp), %rax
        movl    40(%rax), %eax
-       movzwl  %ax, %eax
        movl    %eax, -72(%rbp)
        cmpl    $83, -108(%rbp)
        jne     .L601
@@ -8991,8 +8989,7 @@
        movl    $458752, -12(%rbp)
 .L603:
        movq    -104(%rbp), %rax
-       movl    32(%rax), %eax
-       movzwl  %ax, %edi
+       movl    32(%rax), %edi
        movl    -72(%rbp), %esi
        movl    -76(%rbp), %ecx
        movzwl  -26(%rbp), %edx
(and so on.)

2. The get_unaligned_le32() changes produce new pointer offsets in the 
assembly code for qla82xx_get_table_desc() and qla82xx_get_data_desc().

diff -ru /tmp/unpatched/qla_target.s /tmp/patched/qla_target.s
--- /tmp/unpatched/qla_target.s 2020-05-24 14:02:32.178019380 +1000
+++ /tmp/patched/qla_target.s   2020-05-24 14:01:43.487947966 +1000
@@ -12884,10 +12884,10 @@
        .cfi_offset 6, -16
        movq    %rsp, %rbp
        .cfi_def_cfa_register 6
-       subq    $32, %rsp
-       movq    %rdi, -24(%rbp)
-       movq    %rsi, -32(%rbp)
-       movq    -32(%rbp), %rax
+       subq    $64, %rsp
+       movq    %rdi, -56(%rbp)
+       movq    %rsi, -64(%rbp)
+       movq    -64(%rbp), %rax
        movl    52(%rax), %eax
        movl    %eax, -8(%rbp)
        movl    $24, -12(%rbp)
@@ -12895,62 +12895,62 @@
        cmpl    %eax, -12(%rbp)
        cmovbe  -12(%rbp), %eax
        movl    %eax, %edx
-       movq    -32(%rbp), %rax
+       movq    -64(%rbp), %rax
        movl    %edx, 52(%rax)
-       movq    -24(%rbp), %rax
+       movq    -56(%rbp), %rax
(and so on.)

Was this expected? I find it surprising...

FWIW, I think that the .s files could have been validated automatically 
had the changes in the patch been confined to annotations.