* [PATCH, VAX] Correct ffs instruction constraint
@ 2017-06-20 20:06 coypu
2017-06-29 15:47 ` coypu
0 siblings, 1 reply; 6+ messages in thread
From: coypu @ 2017-06-20 20:06 UTC (permalink / raw)
To: gcc-patches; +Cc: matt, m4j0rd0m0, paulkoning, jbglaw
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH, VAX] Correct ffs instruction constraint
2017-06-20 20:06 [PATCH, VAX] Correct ffs instruction constraint coypu
@ 2017-06-29 15:47 ` coypu
2017-06-29 19:44 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: coypu @ 2017-06-29 15:47 UTC (permalink / raw)
To: gcc-patches; +Cc: matt
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, VAX] Correct ffs instruction constraint
2017-06-29 15:47 ` coypu
@ 2017-06-29 19:44 ` Jeff Law
2017-07-06 17:00 ` Felix Deichmann
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2017-06-29 19:44 UTC (permalink / raw)
To: coypu, gcc-patches; +Cc: matt
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, VAX] Correct ffs instruction constraint
2017-06-29 19:44 ` Jeff Law
@ 2017-07-06 17:00 ` Felix Deichmann
2017-07-06 18:53 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: Felix Deichmann @ 2017-07-06 17:00 UTC (permalink / raw)
To: law; +Cc: coypu, gcc-patches, matt
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, VAX] Correct ffs instruction constraint
2017-07-06 17:00 ` Felix Deichmann
@ 2017-07-06 18:53 ` Jeff Law
2017-07-07 14:55 ` Felix Deichmann
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2017-07-06 18:53 UTC (permalink / raw)
To: Felix Deichmann; +Cc: coypu, gcc-patches, matt
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, VAX] Correct ffs instruction constraint
2017-07-06 18:53 ` Jeff Law
@ 2017-07-07 14:55 ` Felix Deichmann
0 siblings, 0 replies; 6+ messages in thread
From: Felix Deichmann @ 2017-07-07 14:55 UTC (permalink / raw)
To: Jeff Law; +Cc: coypu, gcc-patches, matt
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-07-07 14:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-20 20:06 [PATCH, VAX] Correct ffs instruction constraint coypu
2017-06-29 15:47 ` coypu
2017-06-29 19:44 ` Jeff Law
2017-07-06 17:00 ` Felix Deichmann
2017-07-06 18:53 ` Jeff Law
2017-07-07 14:55 ` Felix Deichmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).