| Message ID | 20170620200542.GA17979@SDF.ORG |
|---|---|
| State | New |
| Headers | show |
Ping. On Tue, Jun 20, 2017 at 08:05:42PM +0000, coypu@sdf.org wrote: > VAX' FFS as variable-length bit field instruction uses a "base" > operand of type "vb" meaning "byte address". > "base" can be 32 bits (SI) and due to the definition of > ffssi2/__builtin_ffs() with the operand constraint "m", code can be > emitted which incorrectly implies a mode-dependent (= longword, for > the 32-bit operand) address. > File scsipi_base.c compiled with -Os for our VAX install kernel shows: > > ffs $0x0,$0x20,0x50(r11)[r0],r9 > > Apparently, 0x50(r11)[r0] as a longword address is assumed to be > evaluated in longword context by FFS, but the instruction expects a > byte address. > > Our fix is to change the operand constraint from "m" to "Q", i. e. > "operand is a MEM that does not have a mode-dependent address", which > results in: > > moval 0x50(r11)[r0],r1 > ffs $0x0,$0x20,(r1),r9 > > MOVAL evaluates the source operand/address in longword context, so > effectively converts the word address to a byte address for FFS. > > See NetBSD PR port-vax/51761 (http://gnats.netbsd.org/51761) and > discussion on port-vax mailing list > (http://mail-index.netbsd.org/port-vax/2017/01/06/msg002954.html). > > Changlog: > > 2017-06-20 Maya Rashish <coypu@sdf.org> > > * gcc/config/vax/builtins.md: Correct ffssi2_internal > instruction constraint. > > > --- > gcc/config/vax/builtins.md | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/config/vax/builtins.md b/gcc/config/vax/builtins.md > index fb0f69acb..b78fb5616 100644 > --- a/gcc/config/vax/builtins.md > +++ b/gcc/config/vax/builtins.md > @@ -41,7 +41,7 @@ > > (define_insn "ffssi2_internal" > [(set (match_operand:SI 0 "nonimmediate_operand" "=rQ") > - (ffs:SI (match_operand:SI 1 "general_operand" "nrmT"))) > + (ffs:SI (match_operand:SI 1 "general_operand" "nrQT"))) > (set (cc0) (match_dup 0))] > "" > "ffs $0,$32,%1,%0") > -- > 2.13.1
On 06/29/2017 09:47 AM, coypu@sdf.org wrote: > Ping. > > On Tue, Jun 20, 2017 at 08:05:42PM +0000, coypu@sdf.org wrote: >> VAX' FFS as variable-length bit field instruction uses a "base" >> operand of type "vb" meaning "byte address". >> "base" can be 32 bits (SI) and due to the definition of >> ffssi2/__builtin_ffs() with the operand constraint "m", code can be >> emitted which incorrectly implies a mode-dependent (= longword, for >> the 32-bit operand) address. >> File scsipi_base.c compiled with -Os for our VAX install kernel shows: >> >> ffs $0x0,$0x20,0x50(r11)[r0],r9 >> >> Apparently, 0x50(r11)[r0] as a longword address is assumed to be >> evaluated in longword context by FFS, but the instruction expects a >> byte address. >> >> Our fix is to change the operand constraint from "m" to "Q", i. e. >> "operand is a MEM that does not have a mode-dependent address", which >> results in: >> >> moval 0x50(r11)[r0],r1 >> ffs $0x0,$0x20,(r1),r9 >> >> MOVAL evaluates the source operand/address in longword context, so >> effectively converts the word address to a byte address for FFS. >> >> See NetBSD PR port-vax/51761 (http://gnats.netbsd.org/51761) and >> discussion on port-vax mailing list >> (http://mail-index.netbsd.org/port-vax/2017/01/06/msg002954.html). >> >> Changlog: >> >> 2017-06-20 Maya Rashish <coypu@sdf.org> >> >> * gcc/config/vax/builtins.md: Correct ffssi2_internal >> instruction constraint. Thanks. Installed. Ideally we'd like to have a testcase for this in the regression suite. If you could provide the .i file and options used which generated the incorrect ffs instruction I can use the reduction tools with a cross compiler to produce a nice simple test for the testsuite. jeff
Jeff, Am 29.06.2017 schrieb Jeff Law <law@redhat.com>: > Ideally we'd like to have a testcase for this in the regression suite. > > If you could provide the .i file and options used which generated the > incorrect ffs instruction I can use the reduction tools with a cross > compiler to produce a nice simple test for the testsuite. I put the corresponding .i file at: http://www.netbsd.org/~flxd/scsipi_base.i.gz See line 7638: bit = __builtin_ffs(periph->periph_freetags[word]); Command/Options used which generated the incorrect ffs instruction: /nb8/obj/tooldir.NetBSD-7.0-amd64/bin/vax--netbsdelf-gcc -fno-pic -ffreestanding -fno-zero-initialized-in-bss -Os -fno-strict-aliasing -fno-common -std=gnu99 -Werror -Wall -Wno-main -Wno-format-zero-length -Wpointer-arith -Wmissing-prototypes -Wstrict-prototypes -Wold-style-definition -Wswitch -Wshadow -Wcast-qual -Wwrite-strings -Wno-unreachable-code -Wno-pointer-sign -Wno-attributes -Wno-sign-compare --sysroot=/nb8/obj/destdir.vax -D_VAX_INLINE_ -I. -I/nb8/src/sys/../common/lib/libx86emu -I/nb8/src/sys/../common/include -I/nb8/src/sys/arch -I/nb8/src/sys -nostdinc -D_KERNEL -D_KERNEL_OPT -std=gnu99 -I/nb8/src/sys/lib/libkern/../../../common/lib/libc/quad -I/nb8/src/sys/lib/libkern/../../../common/lib/libc/string -I/nb8/src/sys/lib/libkern/../../../common/lib/libc/arch/vax/string -c /nb8/src/sys/dev/scsipi/scsipi_base.c -o scsipi_base.o Best regards, Felix
On 07/06/2017 10:59 AM, Felix Deichmann wrote: > Jeff, > > Am 29.06.2017 schrieb Jeff Law <law@redhat.com>: >> Ideally we'd like to have a testcase for this in the regression suite. >> >> If you could provide the .i file and options used which generated the >> incorrect ffs instruction I can use the reduction tools with a cross >> compiler to produce a nice simple test for the testsuite. > > I put the corresponding .i file at: > http://www.netbsd.org/~flxd/scsipi_base.i.gz > > See line 7638: > bit = __builtin_ffs(periph->periph_freetags[word]); > > Command/Options used which generated the incorrect ffs instruction: > > /nb8/obj/tooldir.NetBSD-7.0-amd64/bin/vax--netbsdelf-gcc -fno-pic > -ffreestanding -fno-zero-initialized-in-bss -Os -fno-strict-aliasing > -fno-common -std=gnu99 -Werror -Wall -Wno-main -Wno-format-zero-length > -Wpointer-arith -Wmissing-prototypes -Wstrict-prototypes > -Wold-style-definition -Wswitch -Wshadow -Wcast-qual -Wwrite-strings > -Wno-unreachable-code -Wno-pointer-sign -Wno-attributes > -Wno-sign-compare --sysroot=/nb8/obj/destdir.vax -D_VAX_INLINE_ -I. > -I/nb8/src/sys/../common/lib/libx86emu -I/nb8/src/sys/../common/include > -I/nb8/src/sys/arch -I/nb8/src/sys -nostdinc -D_KERNEL -D_KERNEL_OPT > -std=gnu99 -I/nb8/src/sys/lib/libkern/../../../common/lib/libc/quad > -I/nb8/src/sys/lib/libkern/../../../common/lib/libc/string > -I/nb8/src/sys/lib/libkern/../../../common/lib/libc/arch/vax/string -c > /nb8/src/sys/dev/scsipi/scsipi_base.c -o scsipi_base.o Hmm, unfortunately I consistently get a call to into libgcc for the __builtin_ffs code rather than an ffs instruction. That's with a gcc-4.8.3 as well as with trunk compiler. Can you include "-v" output from compiling scsipi_base? Thanks. jeff
Am 06.07.2017 um 20:53 schrieb Jeff Law: > Hmm, unfortunately I consistently get a call to into libgcc for the > __builtin_ffs code rather than an ffs instruction. That's with a > gcc-4.8.3 as well as with trunk compiler. > > Can you include "-v" output from compiling scsipi_base? Hope this is right/enough: Using built-in specs. COLLECT_GCC=/nb8/obj/tooldir.NetBSD-7.0-amd64/bin/vax--netbsdelf-gcc Target: vax--netbsdelf Configured with: /nb8/src/tools/gcc/../../external/gpl3/gcc/dist/configure --target=vax--netbsdelf --enable-long-long --enable-threads --with-bugurl=http://www.NetBSD.org/Misc/send-pr.html --with-pkgversion='NetBSD nb1 20160606' --with-system-zlib --enable-__cxa_atexit --enable-libstdcxx-time=rt --enable-libstdcxx-threads --with-diagnostics-color=auto-if-env --with-sysroot=/nb8/obj/destdir.vax --with-mpc=/nb8/obj/tooldir.NetBSD-7.0-amd64 --with-mpfr=/nb8/obj/tooldir.NetBSD-7.0-amd64 --with-gmp=/nb8/obj/tooldir.NetBSD-7.0-amd64 --disable-nls --disable-multilib --program-transform-name=s,^,vax--netbsdelf-, --enable-languages='c c++ objc' --prefix=/nb8/obj/tooldir.NetBSD-7.0-amd64 Thread model: posix gcc version 5.4.0 (NetBSD nb1 20160606) COLLECT_GCC_OPTIONS='-v' '-fno-pic' '-ffreestanding' '-fno-zero-initialized-in-bss' '-Os' '-fno-strict-aliasing' '-fno-common' '-std=gnu99' '-Werror' '-Wall' '-Wno-main' '-Wno-format-zero-length' '-Wpointer-arith' '-Wmissing-prototypes' '-Wstrict-prototypes' '-Wold-style-definition' '-Wswitch' '-Wshadow' '-Wcast-qual' '-Wwrite-strings' '-Wno-pointer-sign' '-Wno-attributes' '-Wno-sign-compare' '-D' '_VAX_INLINE_' '-I' '.' '-I' '/nb8/src/sys/../common/lib/libx86emu' '-I' '/nb8/src/sys/../common/include' '-I' '/nb8/src/sys/arch' '-I' '/nb8/src/sys' '-nostdinc' '-D' '_KERNEL' '-D' '_KERNEL_OPT' '-std=gnu99' '-I' '/nb8/src/sys/lib/libkern/../../../common/lib/libc/quad' '-I' '/nb8/src/sys/lib/libkern/../../../common/lib/libc/string' '-I' '/nb8/src/sys/lib/libkern/../../../common/lib/libc/arch/vax/string' '-c' '-o' 'scsipi_base.o' /nb8/obj/tooldir.NetBSD-7.0-amd64/libexec/gcc/vax--netbsdelf/5.4.0/cc1 -quiet -nostdinc -v -I . -I /nb8/src/sys/../common/lib/libx86emu -I /nb8/src/sys/../common/include -I /nb8/src/sys/arch -I /nb8/src/sys -I /nb8/src/sys/lib/libkern/../../../common/lib/libc/quad -I /nb8/src/sys/lib/libkern/../../../common/lib/libc/string -I /nb8/src/sys/lib/libkern/../../../common/lib/libc/arch/vax/string -isysroot /nb8/obj/destdir.vax -D _VAX_INLINE_ -D _KERNEL -D _KERNEL_OPT /nb8/src/sys/dev/scsipi/scsipi_base.c -quiet -dumpbase scsipi_base.c -auxbase-strip scsipi_base.o -Os -Werror -Wall -Wno-main -Wno-format-zero-length -Wpointer-arith -Wmissing-prototypes -Wstrict-prototypes -Wold-style-definition -Wswitch -Wshadow -Wcast-qual -Wwrite-strings -Wno-pointer-sign -Wno-attributes -Wno-sign-compare -std=gnu99 -std=gnu99 -version -fno-pic -ffreestanding -fno-zero-initialized-in-bss -fno-strict-aliasing -fno-common -o /var/tmp//ccy92Bnl.s GNU C99 (NetBSD nb1 20160606) version 5.4.0 (vax--netbsdelf) compiled by GNU C version 4.8.4, GMP version 5.1.3, MPFR version 3.1.2, MPC version 1.0.1 GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072 #include "..." search starts here: #include <...> search starts here: . /nb8/src/sys/../common/lib/libx86emu /nb8/src/sys/../common/include /nb8/src/sys/arch /nb8/src/sys /nb8/src/sys/lib/libkern/../../../common/lib/libc/quad /nb8/src/sys/lib/libkern/../../../common/lib/libc/string /nb8/src/sys/lib/libkern/../../../common/lib/libc/arch/vax/string End of search list. GNU C99 (NetBSD nb1 20160606) version 5.4.0 (vax--netbsdelf) compiled by GNU C version 4.8.4, GMP version 5.1.3, MPFR version 3.1.2, MPC version 1.0.1 GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072 Compiler executable checksum: 530755d1b0ada9bef015017ee74733db COLLECT_GCC_OPTIONS='-v' '-fno-pic' '-ffreestanding' '-fno-zero-initialized-in-bss' '-Os' '-fno-strict-aliasing' '-fno-common' '-std=gnu99' '-Werror' '-Wall' '-Wno-main' '-Wno-format-zero-length' '-Wpointer-arith' '-Wmissing-prototypes' '-Wstrict-prototypes' '-Wold-style-definition' '-Wswitch' '-Wshadow' '-Wcast-qual' '-Wwrite-strings' '-Wno-pointer-sign' '-Wno-attributes' '-Wno-sign-compare' '-D' '_VAX_INLINE_' '-I' '.' '-I' '/nb8/src/sys/../common/lib/libx86emu' '-I' '/nb8/src/sys/../common/include' '-I' '/nb8/src/sys/arch' '-I' '/nb8/src/sys' '-nostdinc' '-D' '_KERNEL' '-D' '_KERNEL_OPT' '-std=gnu99' '-I' '/nb8/src/sys/lib/libkern/../../../common/lib/libc/quad' '-I' '/nb8/src/sys/lib/libkern/../../../common/lib/libc/string' '-I' '/nb8/src/sys/lib/libkern/../../../common/lib/libc/arch/vax/string' '-c' '-o' 'scsipi_base.o' /nb8/obj/tooldir.NetBSD-7.0-amd64/lib/gcc/vax--netbsdelf/5.4.0/../../../../vax--netbsdelf/bin/as -v -I . -I /nb8/src/sys/../common/lib/libx86emu -I /nb8/src/sys/../common/include -I /nb8/src/sys/arch -I /nb8/src/sys -I /nb8/src/sys/lib/libkern/../../../common/lib/libc/quad -I /nb8/src/sys/lib/libkern/../../../common/lib/libc/string -I /nb8/src/sys/lib/libkern/../../../common/lib/libc/arch/vax/string -o scsipi_base.o /var/tmp//ccy92Bnl.s GNU assembler version 2.27 (vax--netbsdelf) using BFD version (NetBSD Binutils nb1) 2.27 COMPILER_PATH=/nb8/obj/tooldir.NetBSD-7.0-amd64/libexec/gcc/vax--netbsdelf/5.4.0/:/nb8/obj/tooldir.NetBSD-7.0-amd64/libexec/gcc/vax--netbsdelf/5.4.0/:/nb8/obj/tooldir.NetBSD-7.0-amd64/libexec/gcc/vax--netbsdelf/:/nb8/obj/tooldir.NetBSD-7.0-amd64/lib/gcc/vax--netbsdelf/5.4.0/:/nb8/obj/tooldir.NetBSD-7.0-amd64/lib/gcc/vax--netbsdelf/:/nb8/obj/tooldir.NetBSD-7.0-amd64/lib/gcc/vax--netbsdelf/5.4.0/../../../../vax--netbsdelf/bin/ LIBRARY_PATH=/nb8/obj/tooldir.NetBSD-7.0-amd64/lib/gcc/vax--netbsdelf/5.4.0/:/nb8/obj/tooldir.NetBSD-7.0-amd64/lib/gcc/vax--netbsdelf/5.4.0/../../../../vax--netbsdelf/lib/:/nb8/obj/destdir.vax/usr/lib/ COLLECT_GCC_OPTIONS='-v' '-fno-pic' '-ffreestanding' '-fno-zero-initialized-in-bss' '-Os' '-fno-strict-aliasing' '-fno-common' '-std=gnu99' '-Werror' '-Wall' '-Wno-main' '-Wno-format-zero-length' '-Wpointer-arith' '-Wmissing-prototypes' '-Wstrict-prototypes' '-Wold-style-definition' '-Wswitch' '-Wshadow' '-Wcast-qual' '-Wwrite-strings' '-Wno-pointer-sign' '-Wno-attributes' '-Wno-sign-compare' '-D' '_VAX_INLINE_' '-I' '.' '-I' '/nb8/src/sys/../common/lib/libx86emu' '-I' '/nb8/src/sys/../common/include' '-I' '/nb8/src/sys/arch' '-I' '/nb8/src/sys' '-nostdinc' '-D' '_KERNEL' '-D' '_KERNEL_OPT' '-std=gnu99' '-I' '/nb8/src/sys/lib/libkern/../../../common/lib/libc/quad' '-I' '/nb8/src/sys/lib/libkern/../../../common/lib/libc/string' '-I' '/nb8/src/sys/lib/libkern/../../../common/lib/libc/arch/vax/string' '-c' '-o' 'scsipi_base.o' Cheers, Felix
diff --git a/gcc/config/vax/builtins.md b/gcc/config/vax/builtins.md index fb0f69acb..b78fb5616 100644 --- a/gcc/config/vax/builtins.md +++ b/gcc/config/vax/builtins.md @@ -41,7 +41,7 @@ (define_insn "ffssi2_internal" [(set (match_operand:SI 0 "nonimmediate_operand" "=rQ") - (ffs:SI (match_operand:SI 1 "general_operand" "nrmT"))) + (ffs:SI (match_operand:SI 1 "general_operand" "nrQT"))) (set (cc0) (match_dup 0))] "" "ffs $0,$32,%1,%0")
VAX' FFS as variable-length bit field instruction uses a "base" operand of type "vb" meaning "byte address". "base" can be 32 bits (SI) and due to the definition of ffssi2/__builtin_ffs() with the operand constraint "m", code can be emitted which incorrectly implies a mode-dependent (= longword, for the 32-bit operand) address. File scsipi_base.c compiled with -Os for our VAX install kernel shows: ffs $0x0,$0x20,0x50(r11)[r0],r9 Apparently, 0x50(r11)[r0] as a longword address is assumed to be evaluated in longword context by FFS, but the instruction expects a byte address. Our fix is to change the operand constraint from "m" to "Q", i. e. "operand is a MEM that does not have a mode-dependent address", which results in: moval 0x50(r11)[r0],r1 ffs $0x0,$0x20,(r1),r9 MOVAL evaluates the source operand/address in longword context, so effectively converts the word address to a byte address for FFS. See NetBSD PR port-vax/51761 (http://gnats.netbsd.org/51761) and discussion on port-vax mailing list (http://mail-index.netbsd.org/port-vax/2017/01/06/msg002954.html). Changlog: 2017-06-20 Maya Rashish <coypu@sdf.org> * gcc/config/vax/builtins.md: Correct ffssi2_internal instruction constraint. --- gcc/config/vax/builtins.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)