Re: [PATCH i386] Introduce __readeflags () and __writeeflags () intrinsics.
On Thu, Dec 5, 2013 at 8:46 AM, Kirill Yukhin kirill.yuk...@gmail.com wrote:
Oh, no. We don't want assembly in this century ;)
Whoops, sorry. I was trying to do it with minimal changes.
I've implemented approach you proposed.
Batch in the bottom.
Bootstrapped. New tests pass.
Is it ok now?
ChangeLog/
* config/i386/i386.c(IX86_BUILTIN_READ_FLAGS): New.
(IX86_BUILTIN_WRITE_FLAGS): Ditto.
(ix86_init_mmx_sse_builtins): Define
__builtin_ia32_writeeflags_u32, __builtin_ia32_writeeflags_u64,
__builtin_ia32_readeflags_u32, __builtin_ia32_readeflags_u64.
(ix86_expand_builtin): Expand them.
* config/i386/ia32intrin.h (__readeflags): New.
(__writeeflags): Ditto.
* gcc/config/i386/i386.md (*pushflmode): Ditto.
(*popflmode1): Ditto.
testsuite/ChangeLog same as initial mail.
--
Thanks, K
gcc/config/i386/i386.c| 26 +
gcc/config/i386/i386.md | 17
gcc/config/i386/ia32intrin.h | 33 ++
gcc/testsuite/gcc.target/i386/readeflags-1.c | 40
+++
gcc/testsuite/gcc.target/i386/writeeflags-1.c | 30
5 files changed, 146 insertions(+)
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 21963bb..f681346 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -27909,6 +27909,10 @@ enum ix86_builtins
IX86_BUILTIN_CPU_IS,
IX86_BUILTIN_CPU_SUPPORTS,
+ /* Read/write FLAGS register built-ins. */
+ IX86_BUILTIN_READ_FLAGS,
+ IX86_BUILTIN_WRITE_FLAGS,
+
IX86_BUILTIN_MAX
};
@@ -29750,6 +29754,17 @@ ix86_init_mmx_sse_builtins (void)
UCHAR_FTYPE_UCHAR_ULONGLONG_ULONGLONG_PULONGLONG,
IX86_BUILTIN_ADDCARRYX64);
+ /* Read/write FLAGS. */
+ def_builtin (~OPTION_MASK_ISA_64BIT, __builtin_ia32_readeflags_u32,
+ UNSIGNED_FTYPE_VOID, IX86_BUILTIN_READ_FLAGS);
+ def_builtin (OPTION_MASK_ISA_64BIT, __builtin_ia32_readeflags_u64,
+ UINT64_FTYPE_VOID, IX86_BUILTIN_READ_FLAGS);
+ def_builtin (~OPTION_MASK_ISA_64BIT, __builtin_ia32_writeeflags_u32,
+ VOID_FTYPE_UNSIGNED, IX86_BUILTIN_WRITE_FLAGS);
+ def_builtin (OPTION_MASK_ISA_64BIT, __builtin_ia32_writeeflags_u64,
+ VOID_FTYPE_UINT64, IX86_BUILTIN_WRITE_FLAGS);
+
+
/* Add FMA4 multi-arg argument instructions */
for (i = 0, d = bdesc_multi_arg; i ARRAY_SIZE (bdesc_multi_arg); i++,
d++)
{
@@ -33378,6 +33393,17 @@ addcarryx:
emit_insn (gen_rtx_SET (VOIDmode, target, pat));
return target;
+case IX86_BUILTIN_READ_FLAGS:
+ emit_insn (gen_push (gen_rtx_REG (CCmode, FLAGS_REG)));
The FLAGS_REG shuold be generated in an integer mode, appropriate for the push!
+ emit_insn (gen_pop (target));
+ return target;
Please note that target can be null, so you need to generate a
register and move insn in this case. Please see many examples in
i386.c expander code.
+case IX86_BUILTIN_WRITE_FLAGS:
+ arg0 = CALL_EXPR_ARG (exp, 0);
+ emit_insn (gen_push (expand_normal (arg0)));
This expand normal is too simple, you need to check predicate and move
argument to a mode register. Also, there are many examples througout
i386.c expanders.
+ emit_insn (gen_pop (gen_rtx_REG (CCmode, FLAGS_REG)));
Again, FLAGS_REG should be generated in a correct mode. I wonder if
flags_reg_operand correctly checks operand mode ...
+ return 0;
+
case IX86_BUILTIN_GATHERSIV2DF:
icode = CODE_FOR_avx2_gathersiv2df;
goto gather_gen;
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 6976124..1c6b06d 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -1714,6 +1714,23 @@
pop{imodesuffix}\t%0
[(set_attr type pop)
(set_attr mode MODE)])
+
+(define_insn *pushflmode
+ [(set (match_operand:DWIH 0 push_operand =)
+ (match_operand:DWIH 1 flags_reg_operand))]
+
+ pushf{imodesuffix}
+ [(set_attr type push)
+ (set_attr mode MODE)])
+
+(define_insn *popflmode1
You can remove trailing 1 here ...
Uros.
[PATCH] S/390: Function hotpatching option and function attribute
Hi, Andreas Krebbel an I have ported the function hotpatching feature from i386 (aka ms_hook_prologue attribute) to S/390. Andreas has already internally approved the attached patch and will commit it soon (for legal reasons he has to do that himself). The attached patch introduces command line options as well as a function attribute to control the function hotpatching prologue: -m[no-]hotpatch Enable or disable hotpatching prologues for all functions in the compilation unit (with the default prologue size). -mhotpatch=n Same as -mhotpatch, but the size of the prologue is n+2 halfwords (i.e. the trampoline area is n halwords). __attribute__ ((hotpatch)) and __attribute__ ((hotpatch(n))) Work like the option, but only for the specific function. If the attribute and one of the options is given, the attribute always wins. n above is limited to values 0 through 100 (default 12). Functions that are explicitly inline cannot be hotpatched, and hotpatched functions are never implicitly inlined. The layout of a hotpatchable function prologue is nopr %r7 (n times, 2 bytes each, aka trampoline area) function_label: nop 0 (four bytes) The function label alignment is changed to eight bytes to make sure that the eight bytes directly in front of the label reside in the same cacheline. To hotpatch a function, the program first writes the address of the new version of the hotpatched function to the eight bytes before the label (four bytes on 32-bit), then writes code to read that address and to jump to it into the trampoline area and then replaces the nop behind the label with a relative backwards jump into the trampoline area. (Some maintenance of the Icache is necessary in this procedure.) If the program can make sure that the patched functions are always close enough to the original in the memory map, the trampoline area can be omitted (n = 0), and a short relative jump to the patched function can be written over the four byte nop. The patch includes a number of test cases that all run without trouble here. The tests validate the syntax of the options an the attribute and check for the correct number of nops. I have verified the correct layout of the trampoline manually (i.e. the correct placement of the nops). Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany 2013-12-04 Dominik Vogt v...@linux.vnet.ibm.com * config/s390/s390.c (s390_hotpatch_trampoline_halfwords_default): New constant (s390_hotpatch_trampoline_halfwords_max): New constant (s390_hotpatch_trampoline_halfwords): New static variable (get_hotpatch_attribute): New function (s390_handle_hotpatch_attribute): New function (s390_attribute_table): New target specific attribute table to implement the hotpatch attribute (s390_option_override): Parse hotpatch options (s390_function_num_hotpatch_trampoline_halfwords): New function (s390_can_inline_p): Implement target hook to suppress hotpatching for explicitly inlined functions (s390_asm_output_function_label): Generate hotpatch prologue (TARGET_ATTRIBUTE_TABLE): Define to implement target attribute table (TARGET_CAN_INLINE_P): Define to implement target hook * config/s390/s390.opt (mhotpatch): New options -mhotpatch, -mhotpatch= * config/s390/s390-protos.h (s390_asm_output_function_label): Add prototype * config/s390/s390.h (ASM_OUTPUT_FUNCTION_LABEL): Target specific function label generation for hotpatching (FUNCTION_BOUNDARY): Align functions to eight bytes * doc/extend.texi: Document hotpatch attribute * doc/invoke.texi: Document -mhotpatch option * gcc/testsuite/gcc.target/s390/hotpatch-1.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-2.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-3.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-4.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-5.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-6.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-7.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-8.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-9.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-10.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-11.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-12.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-compile-1.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-compile-2.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-compile-3.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-compile-4.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-compile-5.c: New test * gcc/testsuite/gcc.target/s390/hotpatch-compile-6.c: New test *
Re: RFC ThreadSanitizer tests
On Thu, Dec 05, 2013 at 10:12:36AM +0400, max wrote: Hello, Here is a patch with initial ThreadSanitizer testsuite. It basically adds several tests from upstream LLVM testsuite. It works fine on x86_64 with patch from http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59188 applied. Ok to commit or should we wait for fix for 59188? How costly the tests are? I see that the tests have at most 3 threads, so it hopefully doesn't affect that much parallel testing, but how long does it take to run make check-gcc check-g++ RUNTESTFLAGS=tsan.exp ? How much memory does it need? 2013-12-05 Max Ostapenko m.ostape...@partner.samsung.com One extra space missing before . * c-c++-common/tsan: New folder with tests added. Instead of mentioning the directory in the ChangeLog, mention the individual test files. * c-c++-common/tsan/atomic_stack.c: New test. ... * lib/tsan-dg.exp: New testfiles. : New file. * gcc.dg/tsan/tsan.exp: New testfiles. : New file. * g++.dg/dg.exp: Add tsan directory to the list of folders that are handled specially. * g++.dg/dg.exp: Prune tsan subdirectory. Jakub
Re: .cfi in sanitizer code
On Thu, Dec 05, 2013 at 11:39:17AM +0400, Konstantin Serebryany wrote:
On Wed, Dec 4, 2013 at 6:16 PM, Jakub Jelinek ja...@redhat.com wrote:
On Wed, Dec 04, 2013 at 06:09:56PM +0400, Konstantin Serebryany wrote:
This is a maintenance problem because we can not test if we broke
something during development.
e.g. clang doesn't seem to support -fno-dwarf2-cfi-asm
It does, at least both clang 3.3 (from Fedora 19) and clang
3.4 r194685 (which I've built myself some time ago just to look at the
use-after-return etc. sanitization).
That's not what I see in my build:
% cat asm_test.cc
void foo() {
__asm__ __volatile__(.cfi_adjust_cfa_offset 100);
}
% clang -c asm_test.cc -fno-dwarf2-cfi-asm
% clang -c asm_test.cc
% gcc -c asm_test.cc
% gcc -c asm_test.cc -fno-dwarf2-cfi-asm
asm_test.cc: Assembler messages:
asm_test.cc:2: Error: CFI instruction used without previous .cfi_startproc
%
Try clang -fno-dwarf2-cfi-asm -no-integrated-as then? Guess it's integrated
assembler is buggy and doesn't diagnose obvious errors (which using .cfi_*
directives outside of .cfi_startproc ... .cfi_endproc is).
I can not test the change in tsan/rtl/tsan_rtl_amd64.S properly
because I could not make it fail w/o the change, even with
-fno-dwarf2-cfi-asm
But looks correct.
To make tsan_rtl_amd64.S actually fail, you'd need assembler that doesn't
support .cfi_* directives at all. E.g. RHEL5 gas already supports .cfi_*
directives, but not all of them (.cfi_personality/.cfi_lsda), so CFI
directives can be used by the compiler for C code that doesn't need those
.cfi_* directives, but not for C++.
Jakub
[buildrobot] Re: [PATCH] Split -fisolate-erroneous-paths into two options
On Wed, 2013-12-04 20:19:29 -0700, Jeff Law l...@redhat.com wrote: This patch splits up the erroneous path optimization into two pieces. One which detects NULL dereferences and isolates those paths and a second which detects passing/returning a NULL pointer in cases where an attribute says a non-NULL value is required. [...] This seems to break Go, see eg. http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=50428 : g++ -c -DDEFAULT_TARGET_VERSION=\4.9.0\ -DDEFAULT_TARGET_MACHINE=\i686-pc-linux-gnu\ -DIN_GCC_FRONTEND -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -I. -Igo -I../../../gcc/gcc -I../../../gcc/gcc/go -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include -I/opt/cfarm/mpc/include -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/bid -I../libdecnumber -I../../../gcc/gcc/../libbacktrace-o go/go-lang.o -MT go/go-lang.o -MMD -MP -MF go/.deps/go-lang.TPo ../../../gcc/gcc/go/go-lang.c ../../../gcc/gcc/go/go-lang.c: In function ‘bool go_langhook_post_options(const char**)’: ../../../gcc/gcc/go/go-lang.c:276:27: error: ‘struct gcc_options’ has no member named ‘x_flag_isolate_erroneous_paths’ if (!global_options_set.x_flag_isolate_erroneous_paths) ^ ../../../gcc/gcc/go/go-lang.c:277:20: error: ‘struct gcc_options’ has no member named ‘x_flag_isolate_erroneous_paths’ global_options.x_flag_isolate_erroneous_paths = 0; ^ make[2]: *** [go/go-lang.o] Error 1 MfG, JBG -- Jan-Benedict Glaw jbg...@lug-owl.de +49-172-7608481 Signature of:http://www.chiark.greenend.org.uk/~sgtatham/bugs.html the second : signature.asc Description: Digital signature
Re: .cfi in sanitizer code
On Thu, Dec 05, 2013 at 11:51:12AM +0400, Konstantin Serebryany wrote: Committed upstream: http://llvm.org/viewvc/llvm-project?view=revisionrevision=196480 LGTM, can we commit it after the merge you have already prepared, or do you want to do another merge for it? Alternatively, rather than defining separate CFI_INL* and CFI_* macros, you could just use the same names of macros and just provide different definitions based on #ifdef __ASSEMBLER__, in preprocessed assembly the ones without string literals, in C/C++ with them. Jakub
Re: RFC ThreadSanitizer tests
my 2c: running all upstream tsan tests (ninja check-tsan) takes 10 seconds on my (beefy) machine and requires rather little memory. Of course, you need lots of *virtual* memory. On Thu, Dec 5, 2013 at 12:07 PM, Jakub Jelinek ja...@redhat.com wrote: On Thu, Dec 05, 2013 at 10:12:36AM +0400, max wrote: Hello, Here is a patch with initial ThreadSanitizer testsuite. It basically adds several tests from upstream LLVM testsuite. It works fine on x86_64 with patch from http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59188 applied. Ok to commit or should we wait for fix for 59188? How costly the tests are? I see that the tests have at most 3 threads, so it hopefully doesn't affect that much parallel testing, but how long does it take to run make check-gcc check-g++ RUNTESTFLAGS=tsan.exp ? How much memory does it need? 2013-12-05 Max Ostapenko m.ostape...@partner.samsung.com One extra space missing before . * c-c++-common/tsan: New folder with tests added. Instead of mentioning the directory in the ChangeLog, mention the individual test files. * c-c++-common/tsan/atomic_stack.c: New test. ... * lib/tsan-dg.exp: New testfiles. : New file. * gcc.dg/tsan/tsan.exp: New testfiles. : New file. * g++.dg/dg.exp: Add tsan directory to the list of folders that are handled specially. * g++.dg/dg.exp: Prune tsan subdirectory. Jakub
Re: .cfi in sanitizer code
On Thu, Dec 5, 2013 at 12:18 PM, Jakub Jelinek ja...@redhat.com wrote: On Thu, Dec 05, 2013 at 11:51:12AM +0400, Konstantin Serebryany wrote: Committed upstream: http://llvm.org/viewvc/llvm-project?view=revisionrevision=196480 LGTM, can we commit it after the merge you have already prepared, or do you want to do another merge for it? Whichever you prefer is fine. Try clang -fno-dwarf2-cfi-asm -no-integrated-as then? Yes, this did the trick. --kcc Alternatively, rather than defining separate CFI_INL* and CFI_* macros, you could just use the same names of macros and just provide different definitions based on #ifdef __ASSEMBLER__, in preprocessed assembly the ones without string literals, in C/C++ with them. Jakub
Re: RFC ThreadSanitizer tests
On Thu, Dec 05, 2013 at 12:30:21PM +0400, Konstantin Serebryany wrote: my 2c: running all upstream tsan tests (ninja check-tsan) takes 10 seconds on my (beefy) machine and requires rather little memory. Of course, you need lots of *virtual* memory. *virtual* memory is acceptable, not very nice, but we have that already for asan. The thing I was worried was e.g. when libgo testsuite added tests that spawned 1000s of threads and so broke parallel testing by getting close to the max threads per user rlimit (if I remember well). Jakub
Re: [PATCH] Fix for PR59369
On Thu, Dec 05, 2013 at 10:42:35AM +0400, Yury Gribov wrote: This patch fixes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59369 by disabling Linux-specific test on non-Linux platforms. Tested on x86_64-apple-darwin. Ok to commit? Ok, thanks. 2013-12-05 Yury Gribov y.gri...@samsung.com PR sanitizer/59369 * c-c++-common/asan/pr59063-1.c: Disable on non-Linux platforms. * c-c++-common/asan/pr59063-2.c: Likewise. Jakub
Silence class vs. struct warnings (opt_pass, ipa_opt_pass_d)
Hi,
When building GCC on OSX with its native XCode/Clang tools, it outputs
quite some struct X was previously declared as a class or similar
warnings (-Wmismatched-tags is enabled by default).
The attached patch fixes this for class opt_pass and class
ipa_opt_pass_d by removing the redundant 'struct' tag which seems to be
a leftover from the plain C days.
Tested with make all-gcc.
OK for trunk?
Cheers,
Oleg
gcc/ChangeLog:
* cgraphunit.c: Remove struct tags when referring to class
ipa_opt_pass_d or class opt_pass.
* function.h: Likewise.
* lto-cgraph.c: Likewise.
* pass_manager.h: Likewise.
* passes.c: Likewise.
* tree-pass.h: Likewise.
Index: gcc/cgraphunit.c
===
--- gcc/cgraphunit.c (revision 205573)
+++ gcc/cgraphunit.c (working copy)
@@ -2019,7 +2019,7 @@
cgraph_process_new_functions ();
execute_ipa_summary_passes
- ((struct ipa_opt_pass_d *) passes-all_regular_ipa_passes);
+ ((ipa_opt_pass_d *) passes-all_regular_ipa_passes);
}
/* Some targets need to handle LTO assembler output specially. */
Index: gcc/function.h
===
--- gcc/function.h (revision 205573)
+++ gcc/function.h (working copy)
@@ -167,8 +167,8 @@
struct call_site_record_d;
struct dw_fde_struct;
-struct ipa_opt_pass_d;
-typedef struct ipa_opt_pass_d *ipa_opt_pass;
+class ipa_opt_pass_d;
+typedef ipa_opt_pass_d *ipa_opt_pass;
struct GTY(()) varasm_status {
Index: gcc/lto-cgraph.c
===
--- gcc/lto-cgraph.c (revision 205573)
+++ gcc/lto-cgraph.c (working copy)
@@ -389,7 +389,7 @@
intptr_t ref;
bool in_other_partition = false;
struct cgraph_node *clone_of, *ultimate_clone_of;
- struct ipa_opt_pass_d *pass;
+ ipa_opt_pass_d *pass;
int i;
bool alias_p;
@@ -1060,12 +1060,12 @@
node-ipa_transforms_to_apply = vNULL;
for (i = 0; i count; i++)
{
- struct opt_pass *pass;
+ opt_pass *pass;
int pid = streamer_read_hwi (ib);
gcc_assert (pid passes-passes_by_id_size);
pass = passes-passes_by_id[pid];
- node-ipa_transforms_to_apply.safe_push ((struct ipa_opt_pass_d *) pass);
+ node-ipa_transforms_to_apply.safe_push ((ipa_opt_pass_d *) pass);
}
if (tag == LTO_symtab_analyzed_node)
Index: gcc/pass_manager.h
===
--- gcc/pass_manager.h (revision 205573)
+++ gcc/pass_manager.h (working copy)
@@ -51,7 +51,7 @@
pass_manager (context *ctxt);
void register_pass (struct register_pass_info *pass_info);
- void register_one_dump_file (struct opt_pass *pass);
+ void register_one_dump_file (opt_pass *pass);
opt_pass *get_pass_for_id (int id) const;
@@ -91,8 +91,8 @@
private:
void set_pass_for_id (int id, opt_pass *pass);
- int register_dump_files_1 (struct opt_pass *pass, int properties);
- void register_dump_files (struct opt_pass *pass, int properties);
+ int register_dump_files_1 (opt_pass *pass, int properties);
+ void register_dump_files (opt_pass *pass, int properties);
private:
context *m_ctxt;
Index: gcc/passes.c
===
--- gcc/passes.c (revision 205573)
+++ gcc/passes.c (working copy)
@@ -90,9 +90,9 @@
/* This is used for debugging. It allows the current pass to printed
from anywhere in compilation.
The variable current_pass is also used for statistics and plugins. */
-struct opt_pass *current_pass;
+opt_pass *current_pass;
-static void register_pass_name (struct opt_pass *, const char *);
+static void register_pass_name (opt_pass *, const char *);
/* Most passes are single-instance (within their context) and thus don't
need to implement cloning, but passes that support multiple instances
@@ -613,12 +613,12 @@
void
pass_manager::
-set_pass_for_id (int id, struct opt_pass *pass)
+set_pass_for_id (int id, opt_pass *pass)
{
pass-static_pass_number = id;
if (passes_by_id_size = id)
{
- passes_by_id = XRESIZEVEC (struct opt_pass *, passes_by_id, id + 1);
+ passes_by_id = XRESIZEVEC (opt_pass *, passes_by_id, id + 1);
memset (passes_by_id + passes_by_id_size, 0,
(id + 1 - passes_by_id_size) * sizeof (void *));
passes_by_id_size = id + 1;
@@ -628,7 +628,7 @@
/* Return the pass with the static pass number ID. */
-struct opt_pass *
+opt_pass *
pass_manager::get_pass_for_id (int id) const
{
if (id = passes_by_id_size)
@@ -641,13 +641,13 @@
enabled or not. */
void
-register_one_dump_file (struct opt_pass *pass)
+register_one_dump_file (opt_pass *pass)
{
g-get_passes ()-register_one_dump_file (pass);
}
void
-pass_manager::register_one_dump_file (struct opt_pass *pass)
+pass_manager::register_one_dump_file (opt_pass *pass)
{
char *dot_name, *flag_name,
Re: [PING] [PATCH] Optional alternative base_expr in finding basis for CAND_REFs
On Wed, Dec 4, 2013 at 2:27 PM, Bill Schmidt
wschm...@linux.vnet.ibm.com wrote:
On Wed, 2013-12-04 at 07:13 -0600, Bill Schmidt wrote:
On Wed, 2013-12-04 at 11:30 +0100, Richard Biener wrote:
On Wed, Dec 4, 2013 at 11:26 AM, Richard Biener
richard.guent...@gmail.com wrote:
On Tue, Dec 3, 2013 at 11:04 PM, Bill Schmidt
wschm...@linux.vnet.ibm.com wrote:
On Tue, 2013-12-03 at 21:35 +0100, Richard Biener wrote:
Yufeng Zhang yufeng.zh...@arm.com wrote:
On 12/03/13 14:20, Richard Biener wrote:
On Tue, Dec 3, 2013 at 1:50 PM, Yufeng Zhangyufeng.zh...@arm.com
wrote:
On 12/03/13 06:48, Jeff Law wrote:
On 12/02/13 08:47, Yufeng Zhang wrote:
Ping~
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg03360.html
Thanks,
Yufeng
On 11/26/13 15:02, Yufeng Zhang wrote:
On 11/26/13 12:45, Richard Biener wrote:
On Thu, Nov 14, 2013 at 12:25 AM, Yufeng
Zhangyufeng.zh...@arm.com wrote:
On 11/13/13 20:54, Bill Schmidt wrote:
The second version of your original patch is ok with me with
the
following changes. Sorry for the little side adventure into
the
next-interp logic; in the end that's going to hurt more than
it
helps in
this case. Thanks for having a look at it, anyway. Thanks
also for
cleaning up this version to be less intrusive to common
interfaces; I
appreciate it.
Thanks a lot for the review. I've attached an updated patch
with the
suggested changes incorporated.
For the next-interp adventure, I was quite happy to do the
experiment; it's
a good chance of gaining insight into the pass. Many thanks
for
your prompt
replies and patience in guiding!
Everything else looks OK to me. Please ask Richard for final
approval,
as I'm not a maintainer.
First a note, I need to check on voting for Bill as the slsr
maintainer
from the steering committee. Voting was in progress just before
the
close of stage1 development so I haven't tallied the results :-)
Looking forward to some good news! :)
Yes, you are right about the non-trivial 'base' tree are rarely
shared.
The cached is introduced mainly because
get_alternative_base
() may
be
called twice on the same 'base' tree, once in the
find_basis_for_candidate () for look-up and the other time in
alloc_cand_and_find_basis () for record_potential_basis (). I'm
happy
to leave out the cache if you think the benefit is trivial.
Without some sense of how expensive the lookups are vs how often
the
cache hits it's awful hard to know if the cache is worth it.
I'd say take it out unless you have some sense it's really saving
time.
It's a pretty minor implementation detail either way.
I think the affine tree routines are generally expensive; it is
worth having
a cache to avoid calling them too many times. I run the slsr-*.c
tests
under gcc.dg/tree-ssa/ and find out that the cache hit rates range
from
55.6% to 90%, with 73.5% as the average. The samples may not well
represent
the real world scenario, but they do show the fact that the 'base'
tree can
be shared to some extent. So I'd like to have the cache in the
patch.
+/* { dg-do compile } */
+/* { dg-options -O2 -fdump-tree-slsr } */
+
+typedef int arr_2[50][50];
+
+void foo (arr_2 a2, int v1)
+{
+ int i, j;
+
+ i = v1 + 5;
+ j = i;
+ a2 [i-10] [j] = 2;
+ a2 [i] [j++] = i;
+ a2 [i+20] [j++] = i;
+ a2 [i-3] [i-1] += 1;
+ return;
+}
+
+/* { dg-final { scan-tree-dump-times MEM 5 slsr } } */
+/* { dg-final { cleanup-tree-dump slsr } } */
scanning for 5 MEMs looks non-sensical. What transform do
you expect? I see other slsr testcases do similar non-sensical
checking which is bad, too.
As the slsr optimizes CAND_REF candidates by simply lowering
them
to
MEM_REF from e.g. ARRAY_REF, I think scanning for the number of
MEM_REFs
is an effective check. Alternatively, I can add a follow-up
patch to
add some dumping facility in replace_ref () to print out the
replacing
actions when -fdump-tree-slsr-details is on.
I think adding some details to the dump and scanning for them
would
be
better. That's the only change that is required for this to move
forward.
I've updated to patch to dump more details when
-fdump-tree-slsr-details is
on. The tests have also been updated to scan for these new dumps
instead of
MEMs.
I suggest doing it quickly. We're well past stage1 close at this
point.
The bootstrapping on x86_64 is still running. OK to commit if it
succeeds?
I still don't like it. It's using the
Silence class vs. struct warnings (varpool_node)
Hi,
When building GCC on OSX with its native XCode/Clang tools, it outputs
quite some struct X was previously declared as a class or similar
warnings (-Wmismatched-tags is enabled by default).
The attached patch fixes this for class varpool_node by removing the
redundant 'struct' tag which seems to be a leftover from the plain C
days.
Tested with make all-gcc.
OK for trunk?
Cheers,
Oleg
gcc/ChangeLog:
* asan.c: Remove struct tags when referring to class
varpool_node.
* cgraph.h: Likewise.
* cgraphbuild.c: Likewise.
* cgraphunit.c: Likewise.
* cp/decl2.c: Likewise.
* dbxout.c: Likewise.
* dwarf2out.c: Likewise.
* gimple-fold.c: Likewise.
* ipa-devirt.c: Likewise.
* ipa-ref-inline.h: Likewise.
* ipa-ref.h: Likewise.
* ipa-reference.c: Likewise.
* ipa-utils.c: Likewise.
* ipa.c: Likewise.
* lto/lto-partition.c: Likewise.
* lto/lto-symtab.c: Likewise.
* lto/lto.c: Likewise.
* lto-cgraph.c: Likewise.
* lto-streamer-out.c: Likewise.
* lto-streamer.h: Likewise.
* passes.c: Likewise.
* toplev.c: Likewise.
* tree-eh.c: Likewise.
* tree-emutls.c: Likewise.
* tree-pass.h: Likewise.
* tree-ssa-structalias.c: Likewise.
* tree-vectorizer.c: Likewise.
* tree.c: Likewise.
* varasm.c: Likewise.
* varpool.c: Likewise.
Index: gcc/asan.c
===
--- gcc/asan.c (revision 205573)
+++ gcc/asan.c (working copy)
@@ -1634,7 +1634,7 @@
{
/* For static vars if they are known not to be dynamically
initialized, they will be always accessible. */
- struct varpool_node *vnode = varpool_get_node (inner);
+ varpool_node *vnode = varpool_get_node (inner);
if (vnode !vnode-dynamically_initialized)
return;
}
@@ -2214,7 +2214,7 @@
fold_convert (const_ptr_type_node, str_cst));
CONSTRUCTOR_APPEND_ELT (vinner, NULL_TREE,
fold_convert (const_ptr_type_node, module_name_cst));
- struct varpool_node *vnode = varpool_get_node (decl);
+ varpool_node *vnode = varpool_get_node (decl);
int has_dynamic_init = vnode ? vnode-dynamically_initialized : 0;
CONSTRUCTOR_APPEND_ELT (vinner, NULL_TREE,
build_int_cst (uptr, has_dynamic_init));
@@ -2380,7 +2380,7 @@
void
asan_finish_file (void)
{
- struct varpool_node *vnode;
+ varpool_node *vnode;
unsigned HOST_WIDE_INT gcount = 0;
if (shadow_ptr_types[0] == NULL_TREE)
Index: gcc/cgraph.h
===
--- gcc/cgraph.h (revision 205573)
+++ gcc/cgraph.h (working copy)
@@ -483,7 +483,8 @@
veccgraph_node_ptr nodes;
};
-typedef struct varpool_node *varpool_node_ptr;
+class varpool_node;
+typedef varpool_node *varpool_node_ptr;
/* A varpool node set is a collection of varpool nodes. A varpool node
@@ -796,7 +797,7 @@
typedef void (*cgraph_edge_hook)(struct cgraph_edge *, void *);
typedef void (*cgraph_node_hook)(struct cgraph_node *, void *);
-typedef void (*varpool_node_hook)(struct varpool_node *, void *);
+typedef void (*varpool_node_hook)(varpool_node *, void *);
typedef void (*cgraph_2edge_hook)(struct cgraph_edge *, struct cgraph_edge *,
void *);
typedef void (*cgraph_2node_hook)(struct cgraph_node *, struct cgraph_node *,
@@ -910,50 +911,50 @@
varpool_node_set varpool_node_set_new (void);
varpool_node_set_iterator varpool_node_set_find (varpool_node_set,
- struct varpool_node *);
-void varpool_node_set_add (varpool_node_set, struct varpool_node *);
-void varpool_node_set_remove (varpool_node_set, struct varpool_node *);
+ varpool_node *);
+void varpool_node_set_add (varpool_node_set, varpool_node *);
+void varpool_node_set_remove (varpool_node_set, varpool_node *);
void dump_varpool_node_set (FILE *, varpool_node_set);
void debug_varpool_node_set (varpool_node_set);
void free_varpool_node_set (varpool_node_set);
void ipa_discover_readonly_nonaddressable_vars (void);
-bool varpool_externally_visible_p (struct varpool_node *);
+bool varpool_externally_visible_p (varpool_node *);
/* In predict.c */
bool cgraph_maybe_hot_edge_p (struct cgraph_edge *e);
bool cgraph_optimize_for_size_p (struct cgraph_node *);
/* In varpool.c */
-struct varpool_node *varpool_create_empty_node (void);
-struct varpool_node *varpool_node_for_decl (tree);
-struct varpool_node *varpool_node_for_asm (tree asmname);
-void varpool_mark_needed_node (struct varpool_node *);
+varpool_node *varpool_create_empty_node (void);
+varpool_node *varpool_node_for_decl (tree);
+varpool_node *varpool_node_for_asm (tree asmname);
+void varpool_mark_needed_node (varpool_node *);
void debug_varpool (void);
void dump_varpool (FILE *);
-void dump_varpool_node (FILE *, struct varpool_node *);
+void dump_varpool_node (FILE *, varpool_node *);
void varpool_finalize_decl
Re: libsanitizer merge from upstream r196090
On Wed, Dec 4, 2013 at 5:47 PM, H.J. Lu hjl.to...@gmail.com wrote: On Wed, Dec 4, 2013 at 8:41 AM, Ian Lance Taylor i...@google.com wrote: On Wed, Dec 4, 2013 at 8:04 AM, FX fxcoud...@gmail.com wrote: Well, it regresses against 4.8, so it still is a P1 regression. Does anyone care? Well, you’re one of the maintainers of libsanitizer for GCC, so if you do not care about regressions in your code, it makes little sense for GCC (the whole project) to keep libsanitizer. I’ve posted this regression a month ago, it was not addressed. I’m not sure under what specific arrangement libsanitizer was added to GCC, but in general there is a responsibility of maintainers not to break bootstrap in their code. Yes, it’s a cost, and if you are not willing to do it, why did you contribute in the first place? Or is it a “hit and run” approach to maintainership? I believe this is a case where the GCC project gets more benefit from libsanitizer than libsanitizer gets from being part of the GCC project. We should work with the libsanitizer developers to make this work, not just push everything back on them. I think libsanitizer should be disabled automatically if kernel or glibc are too old. BTW, fixincludes should fix the bad kernel header files from SuSE. Indeed - I'll give it a shot. Richard. -- H.J.
Silence class vs. struct warnings (vec)
Hi, When building GCC on OSX with its native XCode/Clang tools, it outputs quite some struct X was previously declared as a class or similar warnings (-Wmismatched-tags is enabled by default). The attached patch fixes a mismatch in struct vec_prefix when referring to struct vec. Tested with make all-gcc. OK for trunk? Cheers, Oleg gcc/ChangeLog: * vec.h (struct vec_prefix): Use struct vec instead of class vec. Index: gcc/vec.h === --- gcc/vec.h (revision 205573) +++ gcc/vec.h (working copy) @@ -1216,7 +1216,7 @@ } private: - friend class vecT, va_heap, vl_ptr; + friend struct vecT, va_heap, vl_ptr; vec_prefix m_header; T m_data[N];
Re: Silence class vs. struct warnings (opt_pass, ipa_opt_pass_d)
On Dec 5, 2013, at 12:48 AM, Oleg Endo oleg.e...@t-online.de wrote: Hi, When building GCC on OSX with its native XCode/Clang tools, it outputs quite some struct X was previously declared as a class or similar warnings (-Wmismatched-tags is enabled by default). The attached patch fixes this for class opt_pass and class ipa_opt_pass_d by removing the redundant 'struct' tag which seems to be a leftover from the plain C days. Tested with make all-gcc. OK for trunk? No I don't think we want this at all. C++ is clear here. In fact we don't turn on werror for stage 1 for this exact reason. Rather it might be better to check if that flag to turn off the warning and use that. Also this warning is a bad warning for standard c++ code; clang is wrong to enable by default. Thanks, Andrew Pinski Cheers, Oleg gcc/ChangeLog: * cgraphunit.c: Remove struct tags when referring to class ipa_opt_pass_d or class opt_pass. * function.h: Likewise. * lto-cgraph.c: Likewise. * pass_manager.h: Likewise. * passes.c: Likewise. * tree-pass.h: Likewise. struct_opt_pass.patch
Re: [buildrobot] Re: [PATCH] Split -fisolate-erroneous-paths into two options
On Thu, Dec 5, 2013 at 9:18 AM, Jan-Benedict Glaw jbg...@lug-owl.de wrote: On Wed, 2013-12-04 20:19:29 -0700, Jeff Law l...@redhat.com wrote: This patch splits up the erroneous path optimization into two pieces. One which detects NULL dereferences and isolates those paths and a second which detects passing/returning a NULL pointer in cases where an attribute says a non-NULL value is required. [...] This seems to break Go, see eg. http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=50428 : g++ -c -DDEFAULT_TARGET_VERSION=\4.9.0\ -DDEFAULT_TARGET_MACHINE=\i686-pc-linux-gnu\ -DIN_GCC_FRONTEND -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -I. -Igo -I../../../gcc/gcc -I../../../gcc/gcc/go -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include -I/opt/cfarm/mpc/include -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/bid -I../libdecnumber -I../../../gcc/gcc/../libbacktrace-o go/go-lang.o -MT go/go-lang.o -MMD -MP -MF go/.deps/go-lang.TPo ../../../gcc/gcc/go/go-lang.c ../../../gcc/gcc/go/go-lang.c: In function ‘bool go_langhook_post_options(const char**)’: ../../../gcc/gcc/go/go-lang.c:276:27: error: ‘struct gcc_options’ has no member named ‘x_flag_isolate_erroneous_paths’ if (!global_options_set.x_flag_isolate_erroneous_paths) ^ ../../../gcc/gcc/go/go-lang.c:277:20: error: ‘struct gcc_options’ has no member named ‘x_flag_isolate_erroneous_paths’ global_options.x_flag_isolate_erroneous_paths = 0; ^ make[2]: *** [go/go-lang.o] Error 1 Seems to me that is a left-over that should have been removed after the non-call-exception stuff was fixed. Richard. MfG, JBG -- Jan-Benedict Glaw jbg...@lug-owl.de +49-172-7608481 Signature of:http://www.chiark.greenend.org.uk/~sgtatham/bugs.html the second :
Re: libsanitizer merge from upstream r196090
On Mon, Dec 02, 2013 at 03:52:09PM +0400, Konstantin Serebryany wrote: Expected ChangeLog entries: === libsanitizer/ChangeLog 2013-12-0X Kostya Serebryany k...@google.com * All source files: Merge from upstream r196090. * tsan/Makefile.am (tsan_files): Added new files. * tsan/Makefile.in: Regenerate. * sanitizer_common/Makefile.am (sanitizer_common_files): Added new fles. * sanitizer_common/Makefile.in: Regenerate. * lsan/Makefile.am (lsan_files): Added new files. * lsan/Makefile.in: Regenerate. === gcc/testsuite/ChangeLog 2013-12-0X Kostya Serebryany k...@google.com * c-c++-common/asan/null-deref-1.c: Update the test to match the fresh asan run-time. Ok. Please do another merge momentarily to bring in the ppc32 and CFI fixes and I'll then start working on some of the portability fixes outside of compiler-rt maintained files. Jakub
[PATCH] Fix PR59374
This fixes PR59374, dependence analysis during BB SLP handles cases
that depend on the order of vectorized stmt generation in which case
it ignores WAR dependencies. But that of course only applies to
references that participate in the vectorization.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
Richard.
2013-12-05 Richard Biener rguent...@suse.de
PR tree-optimization/59374
* tree-vect-data-refs.c (vect_slp_analyze_data_ref_dependence):
Commonize known and unknown dependence case fixing the allowed
read-write dependence case and dropping code that should not
matter.
* gcc.dg/torture/pr59374-1.c: New testcase.
* gcc.dg/torture/pr59374-2.c: Likewise.
Index: gcc/tree-vect-data-refs.c
===
*** gcc/tree-vect-data-refs.c (revision 205623)
--- gcc/tree-vect-data-refs.c (working copy)
*** vect_slp_analyze_data_ref_dependence (st
*** 497,527
/* Unknown data dependence. */
if (DDR_ARE_DEPENDENT (ddr) == chrec_dont_know)
{
! gimple earlier_stmt;
!
! if (dump_enabled_p ())
! {
! dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
!can't determine dependence between );
! dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM, DR_REF (dra));
! dump_printf (MSG_MISSED_OPTIMIZATION, and );
! dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM, DR_REF (drb));
dump_printf (MSG_MISSED_OPTIMIZATION, \n);
! }
!
! /* We do not vectorize basic blocks with write-write dependencies. */
! if (DR_IS_WRITE (dra) DR_IS_WRITE (drb))
! return true;
!
! /* Check that it's not a load-after-store dependence. */
! earlier_stmt = get_earlier_stmt (DR_STMT (dra), DR_STMT (drb));
! if (DR_IS_WRITE (STMT_VINFO_DATA_REF (vinfo_for_stmt (earlier_stmt
! return true;
!
! return false;
}
!
! if (dump_enabled_p ())
{
dump_printf_loc (MSG_NOTE, vect_location,
determined dependence between );
--- 497,513
/* Unknown data dependence. */
if (DDR_ARE_DEPENDENT (ddr) == chrec_dont_know)
{
! if (dump_enabled_p ())
! {
! dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
! can't determine dependence between );
! dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM, DR_REF (dra));
! dump_printf (MSG_MISSED_OPTIMIZATION, and );
! dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM, DR_REF (drb));
dump_printf (MSG_MISSED_OPTIMIZATION, \n);
! }
}
! else if (dump_enabled_p ())
{
dump_printf_loc (MSG_NOTE, vect_location,
determined dependence between );
*** vect_slp_analyze_data_ref_dependence (st
*** 531,579
dump_printf (MSG_NOTE, \n);
}
! /* Do not vectorize basic blocks with write-write dependences. */
if (DR_IS_WRITE (dra) DR_IS_WRITE (drb))
return true;
! /* Check dependence between DRA and DRB for basic block vectorization.
! If the accesses share same bases and offsets, we can compare their
initial
! constant offsets to decide whether they differ or not. In case of a
read-
! write dependence we check that the load is before the store to ensure
that
! vectorization will not change the order of the accesses. */
!
! HOST_WIDE_INT type_size_a, type_size_b, init_a, init_b;
! gimple earlier_stmt;
!
! /* Check that the data-refs have same bases and offsets. If not, we can't
! determine if they are dependent. */
! if (!operand_equal_p (DR_BASE_ADDRESS (dra), DR_BASE_ADDRESS (drb), 0)
! || !dr_equal_offsets_p (dra, drb))
! return true;
!
! /* Check the types. */
! type_size_a = TREE_INT_CST_LOW (TYPE_SIZE_UNIT (TREE_TYPE (DR_REF (dra;
! type_size_b = TREE_INT_CST_LOW (TYPE_SIZE_UNIT (TREE_TYPE (DR_REF (drb;
!
! if (type_size_a != type_size_b
! || !types_compatible_p (TREE_TYPE (DR_REF (dra)),
! TREE_TYPE (DR_REF (drb
! return true;
!
! init_a = TREE_INT_CST_LOW (DR_INIT (dra));
! init_b = TREE_INT_CST_LOW (DR_INIT (drb));
!
! /* Two different locations - no dependence. */
! if (init_a != init_b)
! return false;
!
! /* We have a read-write dependence. Check that the load is before the
store.
When we vectorize basic blocks, vector load can be only before
corresponding scalar load, and vector store can be only after its
corresponding scalar store. So the order of the acceses is preserved in
case the load is before the store. */
! earlier_stmt = get_earlier_stmt (DR_STMT (dra), DR_STMT (drb));
if (DR_IS_READ (STMT_VINFO_DATA_REF (vinfo_for_stmt (earlier_stmt
! return false;
Re: Silence class vs. struct warnings (vec)
On Dec 5, 2013, at 1:00 AM, Oleg Endo oleg.e...@t-online.de wrote: Hi, When building GCC on OSX with its native XCode/Clang tools, it outputs quite some struct X was previously declared as a class or similar warnings (-Wmismatched-tags is enabled by default). The attached patch fixes a mismatch in struct vec_prefix when referring to struct vec. Tested with make all-gcc. OK for trunk? What is this warning trying to do really? I think this is a very bad warning as points out standard code for no reason. Thanks, Andrew Cheers, Oleg gcc/ChangeLog: * vec.h (struct vec_prefix): Use struct vec instead of class vec. class_vec.patch
Re: Make C11 _Alignof return least not greatest alignment for a type (PR c/52023)
On Thu, Dec 5, 2013 at 6:57 AM, Joseph S. Myers jos...@codesourcery.com wrote:
Index: c-family/c-common.c
===
--- c-family/c-common.c (revision 205668)
+++ c-family/c-common.c (working copy)
@@ -4921,14 +4921,17 @@ c_common_get_alias_set (tree t)
}
/* Compute the value of 'sizeof (TYPE)' or '__alignof__ (TYPE)', where
- the second parameter indicates which OPERATOR is being applied.
+ the IS_SIZEOF parameter indicates which operator is being applied.
The COMPLAIN flag controls whether we should diagnose possibly
ill-formed constructs or not. LOC is the location of the SIZEOF or
- TYPEOF operator. */
+ TYPEOF operator. If MIN_ALIGNOF, the least alignment required for
+ a type in any context should be returned, rather than the normal
+ alignment for that type. */
tree
c_sizeof_or_alignof_type (location_t loc,
- tree type, bool is_sizeof, int complain)
+ tree type, bool is_sizeof, bool min_alignof,
+ int complain)
{
const char *op_name;
tree value = NULL;
@@ -4994,6 +4997,22 @@ c_sizeof_or_alignof_type (location_t loc,
value = size_binop_loc (loc, CEIL_DIV_EXPR, TYPE_SIZE_UNIT (type),
size_int (TYPE_PRECISION (char_type_node)
/ BITS_PER_UNIT));
+ else if (min_alignof)
+ {
+ unsigned int align = TYPE_ALIGN (type);
+ align = MIN (align, BIGGEST_ALIGNMENT);
+#ifdef BIGGEST_FIELD_ALIGNMENT
+ align = MIN (align, BIGGEST_FIELD_ALIGNMENT);
+#endif
+ tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
+ type);
+ unsigned int field_align = align;
+#ifdef ADJUST_FIELD_ALIGN
+ field_align = ADJUST_FIELD_ALIGN (field, field_align);
+#endif
Won't *field* be unused if ADJUST_FIELD_ALIGN not defined?
Thanks,
bin
--
Best Regards.
[PATCH] Fix PR56787 testcase fail
This adjusts the testcase to not require float vector division,
maybe the cause the testcase fails on arm and ppc (just a guess).
Installed.
Richard.
2013-12-05 Richard Biener rguent...@suse.de
PR tree-optimization/56787
* gcc.dg/vect/pr56787.c: Adjust to not require vector float
division.
Index: gcc/testsuite/gcc.dg/vect/pr56787.c
===
--- gcc/testsuite/gcc.dg/vect/pr56787.c (revision 205692)
+++ gcc/testsuite/gcc.dg/vect/pr56787.c (working copy)
@@ -5,7 +5,7 @@ inline void
bar (const float s[5], float z[3][5])
{
float a = s[0], b = s[1], c = s[2], d = s[3], e = s[4];
- float f = 1.0f / a;
+ float f = a;
float u = f * b, v = f * c, w = f * d;
float p = 0.4f * (e - 0.5f * (b * u + c * v + d * w));
z[0][3] = b * w;
[buildrobot] Re: Make C11 _Alignof return least not greatest alignment for a type (PR c/52023)
On Thu, 2013-12-05 17:16:53 +0800, Bin.Cheng amker.ch...@gmail.com wrote:
On Thu, Dec 5, 2013 at 6:57 AM, Joseph S. Myers jos...@codesourcery.com
wrote:
Index: c-family/c-common.c
===
--- c-family/c-common.c (revision 205668)
+++ c-family/c-common.c (working copy)
@@ -4921,14 +4921,17 @@ c_common_get_alias_set (tree t)
}
/* Compute the value of 'sizeof (TYPE)' or '__alignof__ (TYPE)', where
- the second parameter indicates which OPERATOR is being applied.
+ the IS_SIZEOF parameter indicates which operator is being applied.
The COMPLAIN flag controls whether we should diagnose possibly
ill-formed constructs or not. LOC is the location of the SIZEOF or
- TYPEOF operator. */
+ TYPEOF operator. If MIN_ALIGNOF, the least alignment required for
+ a type in any context should be returned, rather than the normal
+ alignment for that type. */
tree
c_sizeof_or_alignof_type (location_t loc,
- tree type, bool is_sizeof, int complain)
+ tree type, bool is_sizeof, bool min_alignof,
+ int complain)
{
const char *op_name;
tree value = NULL;
@@ -4994,6 +4997,22 @@ c_sizeof_or_alignof_type (location_t loc,
value = size_binop_loc (loc, CEIL_DIV_EXPR, TYPE_SIZE_UNIT (type),
size_int (TYPE_PRECISION (char_type_node)
/ BITS_PER_UNIT));
+ else if (min_alignof)
+ {
+ unsigned int align = TYPE_ALIGN (type);
+ align = MIN (align, BIGGEST_ALIGNMENT);
+#ifdef BIGGEST_FIELD_ALIGNMENT
+ align = MIN (align, BIGGEST_FIELD_ALIGNMENT);
+#endif
+ tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
+ type);
+ unsigned int field_align = align;
+#ifdef ADJUST_FIELD_ALIGN
+ field_align = ADJUST_FIELD_ALIGN (field, field_align);
+#endif
Won't *field* be unused if ADJUST_FIELD_ALIGN not defined?
You're right:
g++ -c -DIN_GCC_FRONTEND -DIN_GCC_FRONTEND -g -O2 -DIN_GCC
-DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings
-Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long
-Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common
-DHAVE_CONFIG_H -I. -Ic-family -I../../../gcc/gcc -I../../../gcc/gcc/c-family
-I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include
-I/opt/cfarm/mpc/include -I../../../gcc/gcc/../libdecnumber
-I../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber
-I../../../gcc/gcc/../libbacktrace-o c-family/c-common.o -MT
c-family/c-common.o -MMD -MP -MF c-family/.deps/c-common.TPo
../../../gcc/gcc/c-family/c-common.c
../../../gcc/gcc/c-family/c-common.c: In function ‘tree_node*
c_sizeof_or_alignof_type(location_t, tree, bool, bool, int)’:
../../../gcc/gcc/c-family/c-common.c:5007:9: error: unused variable ‘field’
[-Werror=unused-variable]
tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
^
cc1plus: all warnings being treated as errors
make[2]: *** [c-family/c-common.o] Error 1
This is m68k-elf, see eg.
http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=50551 ,
but other targets are also affected.
MfG, JBG
--
Jan-Benedict Glaw jbg...@lug-owl.de +49-172-7608481
Signature of: They that give up essential liberty to obtain temporary safety,
the second : deserve neither liberty nor safety. (Ben Franklin)
signature.asc
Description: Digital signature
Re: Silence class vs. struct warnings (vec)
On Thu, 2013-12-05 at 01:11 -0800, pins...@gmail.com wrote: On Dec 5, 2013, at 1:00 AM, Oleg Endo oleg.e...@t-online.de wrote: Hi, When building GCC on OSX with its native XCode/Clang tools, it outputs quite some struct X was previously declared as a class or similar warnings (-Wmismatched-tags is enabled by default). The attached patch fixes a mismatch in struct vec_prefix when referring to struct vec. Tested with make all-gcc. OK for trunk? What is this warning trying to do really? I think this is a very bad warning as points out standard code for no reason. I think the answer is here: http://llvm.org/bugs/show_bug.cgi?id=11632 Cheers, Oleg
Re: Silence class vs. struct warnings (vec)
On Dec 5, 2013, at 1:33 AM, Oleg Endo oleg.e...@t-online.de wrote: On Thu, 2013-12-05 at 01:11 -0800, pins...@gmail.com wrote: On Dec 5, 2013, at 1:00 AM, Oleg Endo oleg.e...@t-online.de wrote: Hi, When building GCC on OSX with its native XCode/Clang tools, it outputs quite some struct X was previously declared as a class or similar warnings (-Wmismatched-tags is enabled by default). The attached patch fixes a mismatch in struct vec_prefix when referring to struct vec. Tested with make all-gcc. OK for trunk? What is this warning trying to do really? I think this is a very bad warning as points out standard code for no reason. I think the answer is here: http://llvm.org/bugs/show_bug.cgi?id=11632 Except we don't support compiling GCC with microsoft's broken compiler. So again why make this change for broken compilers that is hard to support in the first place? Cheers, Oleg
Re: Fix for PR59368
* Makefile.am (gcc_version): added gcc_version. Capital letter A here. Thanks, fixed. Ok Done in r205698. -Y
Re: [PATCH] Fix for PR59369
Fixed in r205699.
libsanitizer merge from upstream r196489
Another libsanitizer merge from upstream, r196489
(Quick follow up after the r196090 merge)
Fixes (hopefully) .cfi and ppc32 support.
Tested on x86_64 Linux Ubuntu 12.04 box:
make -j 40 -C gcc check-g{cc,++}
RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp'
The ubsan testing fails, but this is unrelated to my change.
The ChangeLog entry:
2013-12-05 Kostya Serebryany k...@google.com
* All source files: Merge from upstream r196489.
* merge.sh: Add *.S to the list of merged files.
--kcc
Index: libsanitizer/sanitizer_common/sanitizer_common.h
===
--- libsanitizer/sanitizer_common/sanitizer_common.h (revision 205696)
+++ libsanitizer/sanitizer_common/sanitizer_common.h (working copy)
@@ -134,6 +134,8 @@
extern bool log_to_file;
extern char report_path_prefix[4096];
extern uptr report_fd_pid;
+extern uptr stoptheworld_tracer_pid;
+extern uptr stoptheworld_tracer_ppid;
uptr OpenFile(const char *filename, bool write);
// Opens the file 'file_name and reads up to 'max_len' bytes.
@@ -318,8 +320,7 @@
class InternalMmapVector {
public:
explicit InternalMmapVector(uptr initial_capacity) {
-CHECK_GT(initial_capacity, 0);
-capacity_ = initial_capacity;
+capacity_ = Max(initial_capacity, (uptr)1);
size_ = 0;
data_ = (T *)MmapOrDie(capacity_ * sizeof(T), InternalMmapVector);
}
Index: libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc
===
--- libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc (revision 205696)
+++ libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc (working copy)
@@ -58,6 +58,22 @@
#define COMMON_INTERCEPTOR_HANDLE_RECVMSG(ctx, msg) ((void)(msg))
#endif
+#if SANITIZER_INTERCEPT_TEXTDOMAIN
+INTERCEPTOR(char*, textdomain, const char *domainname) {
+ void *ctx;
+ COMMON_INTERCEPTOR_ENTER(ctx, textdomain, domainname);
+ char* domain = REAL(textdomain)(domainname);
+ if (domain) {
+COMMON_INTERCEPTOR_INITIALIZE_RANGE(ctx, domain,
+REAL(strlen)(domain) + 1);
+ }
+ return domain;
+}
+#define INIT_TEXTDOMAIN COMMON_INTERCEPT_FUNCTION(textdomain)
+#else
+#define INIT_TEXTDOMAIN
+#endif
+
#if SANITIZER_INTERCEPT_STRCMP
static inline int CharCmpX(unsigned char c1, unsigned char c2) {
return (c1 == c2) ? 0 : (c1 c2) ? -1 : 1;
@@ -2891,6 +2907,7 @@
#endif
#define SANITIZER_COMMON_INTERCEPTORS_INIT \
+ INIT_TEXTDOMAIN; \
INIT_STRCMP; \
INIT_STRNCMP;\
INIT_STRCASECMP; \
Index: libsanitizer/sanitizer_common/sanitizer_asm.h
===
--- libsanitizer/sanitizer_common/sanitizer_asm.h (revision 0)
+++ libsanitizer/sanitizer_common/sanitizer_asm.h (revision 0)
@@ -0,0 +1,36 @@
+//===-- sanitizer_asm.h -*- C++ -*-===//
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Various support for assemebler.
+//
+//===--===//
+
+// Some toolchains do not support .cfi asm directives, so we have to hide
+// them inside macros.
+#if defined(__clang__) || \
+(defined(__GNUC__) defined(__GCC_HAVE_DWARF2_CFI_ASM))
+ // GCC defined __GCC_HAVE_DWARF2_CFI_ASM if it supports CFI.
+ // Clang seems to support CFI by default (or not?).
+ // We need two versions of macros: for inline asm and standalone asm files.
+# define CFI_INL_ADJUST_CFA_OFFSET(n) .cfi_adjust_cfa_offset #n ;
+
+# define CFI_STARTPROC .cfi_startproc
+# define CFI_ENDPROC .cfi_endproc
+# define CFI_ADJUST_CFA_OFFSET(n) .cfi_adjust_cfa_offset n
+# define CFI_REL_OFFSET(reg, n) .cfi_rel_offset reg, n
+# define CFI_DEF_CFA_REGISTER(reg) .cfi_def_cfa_register reg
+# define CFI_RESTORE(reg) .cfi_restore reg
+
+#else // No CFI
+# define CFI_INL_ADJUST_CFA_OFFSET(n)
+# define CFI_STARTPROC
+# define CFI_ENDPROC
+# define CFI_ADJUST_CFA_OFFSET(n)
+# define CFI_REL_OFFSET(reg, n)
+# define CFI_DEF_CFA_REGISTER(reg)
+# define CFI_RESTORE(reg)
+#endif
Index: libsanitizer/sanitizer_common/sanitizer_platform_interceptors.h
===
--- libsanitizer/sanitizer_common/sanitizer_platform_interceptors.h (revision 205696)
+++ libsanitizer/sanitizer_common/sanitizer_platform_interceptors.h (working copy)
@@ -46,6 +46,7 @@
#endif
# define SANITIZER_INTERCEPT_STRCMP 1
+# define SANITIZER_INTERCEPT_TEXTDOMAIN SI_LINUX_NOT_ANDROID
# define SANITIZER_INTERCEPT_STRCASECMP SI_NOT_WINDOWS
# define SANITIZER_INTERCEPT_READ SI_NOT_WINDOWS
Index:
Re: [PATCH, doc] Document -fsanitize=signed-integer-overflow
Ping. The implementation has been commited.
On Thu, Nov 28, 2013 at 01:32:24PM +0100, Marek Polacek wrote:
As promised, this patch on top of this patch by Tobias:
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg03082.html
adds the documentation for -fsanitize=signed-integer-overflow.
Ok to install after the actual implementation is in?
2013-11-28 Marek Polacek pola...@redhat.com
* doc/invoke.texi: Document -fsanitize=signed-integer-overflow.
--- gcc/doc/invoke.texi.mp3 2013-11-28 13:07:09.011575348 +0100
+++ gcc/doc/invoke.texi 2013-11-28 13:24:45.109798224 +0100
@@ -5341,6 +5341,19 @@ built with this option turned on will is
tries to dereference a NULL pointer, or if a reference (possibly an
rvalue reference) is bound to a NULL pointer.
+@item -fsanitize=signed-integer-overflow
+@opindex fsanitize=signed-integer-overflow
+
+This option enables signed integer overflow checking. We check that
+the result of @code{+}, @code{*}, and both unary and binary @code{-}
+does not overflow in the signed arithmetics. Note, integer promotion
+rules must be taken into account. That is, the following is not an
+overflow:
+@smallexample
+signed char a = SCHAR_MAX;
+a++;
+@end smallexample
+
@end table
While @option{-ftrapv} causes traps for signed overflows to be emitted,
Marek
Re: Silence class vs. struct warnings (opt_pass, ipa_opt_pass_d)
On Thu, 2013-12-05 at 01:00 -0800, pins...@gmail.com wrote: No I don't think we want this at all. C++ is clear here. In fact we don't turn on werror for stage 1 for this exact reason. Rather it might be better to check if that flag to turn off the warning and use that. Also this warning is a bad warning for standard c++ code; clang is wrong to enable by default. Yes, warnings have to be disabled when compiling GCC, since clang complains about many more things. Anyway, these issues aside ... No I don't think we want this at all ... why is that? What's the purpose/benefit in C++ of repeatedly writing struct X* if X is already a known type? Cheers, Oleg
Re: libsanitizer merge from upstream r196489
On Thu, Dec 05, 2013 at 02:06:52PM +0400, Konstantin Serebryany wrote:
Another libsanitizer merge from upstream, r196489
(Quick follow up after the r196090 merge)
Fixes (hopefully) .cfi and ppc32 support.
Tested on x86_64 Linux Ubuntu 12.04 box:
make -j 40 -C gcc check-g{cc,++}
RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp'
The ubsan testing fails, but this is unrelated to my change.
The ChangeLog entry:
2013-12-05 Kostya Serebryany k...@google.com
* All source files: Merge from upstream r196489.
* merge.sh: Add *.S to the list of merged files.
Ok, thanks.
Jakub
[PATCH] Fixinclude linux/vt.h problem breaking libsanitizer
This fixes the issue in linux/vt.h that appears in SUSE SLE11 kernel
headers which contain a pre-release variant that is broken and not
compatible with C++ (using the 'new' keyword).
The following fix simply replaces that (and only that) field with
'newev', the upstream choice.
fixinclude-tested and also built libsanitizer on a broken system.
Ok for trunk?
Thanks,
Richard.
2013-12-05 Richard Biener rguent...@suse.de
fixincludes/
* inclhack.def (suse_linux_vt_cxx): New fix for linux/vt.h
being not compatible with C++.
* tests/base/linux/vt.h: New test.
Index: fixincludes/inclhack.def
===
*** fixincludes/inclhack.def(revision 205695)
--- fixincludes/inclhack.def(working copy)
*** fix = {
*** 4828,4831
--- 4828,4845
test_text = #define __builtin_warning(x, y...) (1);
};
+ /*
+ * Linux kernel's vt.h breaks C++
+ */
+ fix = {
+ hackname = suse_linux_vt_cxx;
+ files = linux/vt.h;
+
+ select= ^[ \t]*unsigned int new;;
+ c_fix = format;
+ c_fix_arg = unsigned int newev;;
+
+ test_text = unsigned int new; /* New console (if
changing) */;
+ };
+
/*EOF*/
Index: fixincludes/tests/base/linux/vt.h
===
*** fixincludes/tests/base/linux/vt.h (revision 0)
--- fixincludes/tests/base/linux/vt.h (working copy)
***
*** 0
--- 1,14
+ /* DO NOT EDIT THIS FILE.
+
+ It has been auto-edited by fixincludes from:
+
+ fixinc/tests/inc/linux/vt.h
+
+ This had to be done to correct non-standard usages in the
+ original, manufacturer supplied header file. */
+
+
+
+ #if defined( SUSE_LINUX_VT_CXX_CHECK )
+ unsigned int newev; /* New console (if changing) */
+ #endif /* SUSE_LINUX_VT_CXX_CHECK */
Re: RFC ThreadSanitizer tests
Instead of mentioning the directory in the ChangeLog,
mention the individual test files.
...
* g++.dg/dg.exp: Prune tsan subdirectory.
Thanks, I fixed the ChangeLog file.
how long does it take to run make check-gcc check-g++
RUNTESTFLAGS=tsan.exp ?
How much memory does it need?
I've run `make check-gcc RUNTESTFLAGS=tsan.exp' (this also includes c++
tests) under `/usr/bin/time -v' and got:
-Elapsed (wall clock) time (h:mm:ss or m:ss): 3:17.04
-Maximum resident set size (kbytes): 199872 kbytes
The same numbers for asan are (/usr/bin/time -v make check-gcc
RUNTESTFLAGS=asan.exp):
-Elapsed (wall clock) time (h:mm:ss or m:ss): 3:56.38
-Maximum resident set size (kbytes): 3155264 kbytes
-Maxim
2013-12-05 Max Ostapenko m.ostape...@partner.samsung.com
* c-c++-common/tsan/atomic_stack.c: New test.
* c-c++-common/tsan/fd_pipe_race.c: New test.
* c-c++-common/tsan/free_race.c: New test.
* c-c++-common/tsan/mutexset1.c: New test.
* c-c++-common/tsan/race_on_barrier.c: New test.
* c-c++-common/tsan/sleep_sync.c: New test.
* c-c++-common/tsan/thread_leak.c: New test.
* c-c++-common/tsan/thread_leak1.c: New test.
* c-c++-common/tsan/thread_leak2.c: New test.
* c-c++-common/tsan/tiny_race.c: New test.
* c-c++-common/tsan/tls_race.c: New test.
* c-c++-common/tsan/write_in_reader_lock.c: New test.
* lib/tsan-dg.exp: New file.
* gcc.dg/tsan/tsan.exp: New file.
* g++.dg/tsan/tsan.exp: New file.
* g++.dg/dg.exp: Prune tsan subdirectory.
diff --git a/gcc/testsuite/c-c++-common/tsan/atomic_stack.c b/gcc/testsuite/c-c++-common/tsan/atomic_stack.c
new file mode 100644
index 000..eac71b8
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/tsan/atomic_stack.c
@@ -0,0 +1,32 @@
+/* { dg-do run } */
+/* { dg-shouldfail tsan } */
+
+#include pthread.h
+#include unistd.h
+
+int Global;
+
+void *Thread1(void *x) {
+ sleep(1);
+ __atomic_fetch_add(Global, 1, __ATOMIC_RELAXED);
+ return NULL;
+}
+
+void *Thread2(void *x) {
+ Global++;
+ return NULL;
+}
+
+int main() {
+ pthread_t t[2];
+ pthread_create(t[0], NULL, Thread1, NULL);
+ pthread_create(t[1], NULL, Thread2, NULL);
+ pthread_join(t[0], NULL);
+ pthread_join(t[1], NULL);
+ return 0;
+}
+
+/* { dg-output WARNING: ThreadSanitizer: data race.*(\n|\r\n|\r) } */
+/* { dg-output Atomic write of size 4.* } */
+/* { dg-output #0 __tsan_atomic32_fetch_add.* } */
+/* { dg-output #1 Thread1.* } */
diff --git a/gcc/testsuite/c-c++-common/tsan/fd_pipe_race.c b/gcc/testsuite/c-c++-common/tsan/fd_pipe_race.c
new file mode 100644
index 000..fc76cbf
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/tsan/fd_pipe_race.c
@@ -0,0 +1,37 @@
+/* { dg-do run } */
+/* { dg-shouldfail tsan } */
+
+#include pthread.h
+#include stdio.h
+#include unistd.h
+
+int fds[2];
+
+void *Thread1(void *x) {
+ write(fds[1], a, 1);
+ return NULL;
+}
+
+void *Thread2(void *x) {
+ sleep(1);
+ close(fds[0]);
+ close(fds[1]);
+ return NULL;
+}
+
+int main() {
+ pipe(fds);
+ pthread_t t[2];
+ pthread_create(t[0], NULL, Thread1, NULL);
+ pthread_create(t[1], NULL, Thread2, NULL);
+ pthread_join(t[0], NULL);
+ pthread_join(t[1], NULL);
+}
+
+/* { dg-output WARNING: ThreadSanitizer: data race.*\n } */
+/* { dg-output Write of size 8.*\n } */
+/* { dg-output #0 close.*\n } */
+/* { dg-output #1 Thread2.*\n } */
+/* { dg-output Previous read of size 8.*\n } */
+/* { dg-output #0 write.*\n } */
+/* { dg-output #1 Thread1.*\n } */
diff --git a/gcc/testsuite/c-c++-common/tsan/free_race.c b/gcc/testsuite/c-c++-common/tsan/free_race.c
new file mode 100644
index 000..362c92b
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/tsan/free_race.c
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+/* { dg-shouldfail tsan } */
+
+#include stdlib.h
+
+void __attribute__((noinline)) foo(int *mem) {
+ free(mem);
+}
+
+void __attribute__((noinline)) bar(int *mem) {
+ mem[0] = 42;
+}
+
+int main() {
+ int *mem =(int*)malloc (100);
+ foo(mem);
+ bar(mem);
+ return 0;
+}
+
+/* { dg-output WARNING: ThreadSanitizer: heap-use-after-free.*(\n|\r\n|\r) } */
+/* { dg-output Write of size 4 at.* by main thread:(\n|\r\n|\r) } */
+/* { dg-output #0 bar.*(\n|\r\n|\r) } */
+/* { dg-output #1 main.*(\n|\r\n|\r) } */
+/* { dg-output Previous write of size 8 at.* by main thread:(\n|\r\n|\r) } */
+/* { dg-output #0 free.*(\n|\r\n|\r) } */
+/* { dg-output #\(1|2\) foo.*(\n|\r\n|\r) } */
+/* { dg-output #\(2|3\) main.*(\n|\r\n|\r) } */
diff --git a/gcc/testsuite/c-c++-common/tsan/mutexset1.c b/gcc/testsuite/c-c++-common/tsan/mutexset1.c
new file mode 100644
index 000..783f262
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/tsan/mutexset1.c
@@ -0,0 +1,41 @@
+/* { dg-do run } */
+/* { dg-shouldfail tsan } */
+
+#include pthread.h
+#include stdio.h
+#include unistd.h
+
+int Global;
+pthread_mutex_t mtx;
+
+void *Thread1(void *x) {
+ sleep(1);
+
Re: [PATCH i386] Introduce __readeflags () and __writeeflags () intrinsics.
On 05 Dec 09:01, Uros Bizjak wrote:
On Thu, Dec 5, 2013 at 8:46 AM, Kirill Yukhin kirill.yuk...@gmail.com wrote:
+ emit_insn (gen_push (gen_rtx_REG (CCmode, FLAGS_REG)));
The FLAGS_REG shuold be generated in an integer mode, appropriate for the
push!
I suppose, `word_mode' is correct mode throughout these built-ins expanding.
+ emit_insn (gen_pop (target));
+ return target;
Please note that target can be null, so you need to generate a
register and move insn in this case. Please see many examples in
i386.c expander code.
Fixed.
+case IX86_BUILTIN_WRITE_FLAGS:
+ arg0 = CALL_EXPR_ARG (exp, 0);
+ emit_insn (gen_push (expand_normal (arg0)));
This expand normal is too simple, you need to check predicate and move
argument to a mode register. Also, there are many examples througout
i386.c expanders.
Fixed using `push_operand' predicate.
+ emit_insn (gen_pop (gen_rtx_REG (CCmode, FLAGS_REG)));
Again, FLAGS_REG should be generated in a correct mode. I wonder if
flags_reg_operand correctly checks operand mode ...
Fixed.
+(define_insn *popflmode1
You can remove trailing 1 here ...
Fixed.
Bootstrapped. Tests still pass.
Changelogs untouched.
Is it ok?
--
Thanks, K
gcc/config/i386/i386.c| 37 +
gcc/config/i386/i386.md | 17
gcc/config/i386/ia32intrin.h | 33 ++
gcc/testsuite/gcc.target/i386/readeflags-1.c | 40 +++
gcc/testsuite/gcc.target/i386/writeeflags-1.c | 30
5 files changed, 157 insertions(+)
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 21963bb..5bf358e 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -27909,6 +27909,10 @@ enum ix86_builtins
IX86_BUILTIN_CPU_IS,
IX86_BUILTIN_CPU_SUPPORTS,
+ /* Read/write FLAGS register built-ins. */
+ IX86_BUILTIN_READ_FLAGS,
+ IX86_BUILTIN_WRITE_FLAGS,
+
IX86_BUILTIN_MAX
};
@@ -29750,6 +29754,17 @@ ix86_init_mmx_sse_builtins (void)
UCHAR_FTYPE_UCHAR_ULONGLONG_ULONGLONG_PULONGLONG,
IX86_BUILTIN_ADDCARRYX64);
+ /* Read/write FLAGS. */
+ def_builtin (~OPTION_MASK_ISA_64BIT, __builtin_ia32_readeflags_u32,
+ UNSIGNED_FTYPE_VOID, IX86_BUILTIN_READ_FLAGS);
+ def_builtin (OPTION_MASK_ISA_64BIT, __builtin_ia32_readeflags_u64,
+ UINT64_FTYPE_VOID, IX86_BUILTIN_READ_FLAGS);
+ def_builtin (~OPTION_MASK_ISA_64BIT, __builtin_ia32_writeeflags_u32,
+ VOID_FTYPE_UNSIGNED, IX86_BUILTIN_WRITE_FLAGS);
+ def_builtin (OPTION_MASK_ISA_64BIT, __builtin_ia32_writeeflags_u64,
+ VOID_FTYPE_UINT64, IX86_BUILTIN_WRITE_FLAGS);
+
+
/* Add FMA4 multi-arg argument instructions */
for (i = 0, d = bdesc_multi_arg; i ARRAY_SIZE (bdesc_multi_arg); i++, d++)
{
@@ -33378,6 +33393,28 @@ addcarryx:
emit_insn (gen_rtx_SET (VOIDmode, target, pat));
return target;
+case IX86_BUILTIN_READ_FLAGS:
+ emit_insn (gen_push (gen_rtx_REG (word_mode, FLAGS_REG)));
+
+ if (target == NULL_RTX
+ || !register_operand (target, word_mode)
+ || GET_MODE (target) != word_mode)
+ target = gen_reg_rtx (word_mode);
+
+ emit_insn (gen_pop (target));
+ return target;
+
+case IX86_BUILTIN_WRITE_FLAGS:
+
+ arg0 = CALL_EXPR_ARG (exp, 0);
+ op0 = expand_normal (arg0);
+ if (!push_operand (op0, word_mode))
+ op0 = copy_to_mode_reg (word_mode, op0);
+
+ emit_insn (gen_push (op0));
+ emit_insn (gen_pop (gen_rtx_REG (word_mode, FLAGS_REG)));
+ return 0;
+
case IX86_BUILTIN_GATHERSIV2DF:
icode = CODE_FOR_avx2_gathersiv2df;
goto gather_gen;
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 6976124..272fbdf 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -1714,6 +1714,23 @@
pop{imodesuffix}\t%0
[(set_attr type pop)
(set_attr mode MODE)])
+
+(define_insn *pushflmode
+ [(set (match_operand:DWIH 0 push_operand =)
+ (match_operand:DWIH 1 flags_reg_operand))]
+
+ pushf{imodesuffix}
+ [(set_attr type push)
+ (set_attr mode MODE)])
+
+(define_insn *popflmode
+ [(set (match_operand:DWIH 0 flags_reg_operand)
+ (match_operand:DWIH 1 pop_operand ))]
+
+ popf{imodesuffix}
+ [(set_attr type pop)
+ (set_attr mode MODE)])
+
;; Move instructions.
diff --git a/gcc/config/i386/ia32intrin.h b/gcc/config/i386/ia32intrin.h
index b26dc46..65642e4 100644
--- a/gcc/config/i386/ia32intrin.h
+++ b/gcc/config/i386/ia32intrin.h
@@ -238,6 +238,22 @@ __rorq (unsigned long long __X, int __C)
return (__X __C) | (__X (64 - __C));
}
+/* Read flags register */
+extern __inline unsigned long long
+__attribute__((__gnu_inline__, __always_inline__, __artificial__))
+__readeflags (void)
+{
+ return __builtin_ia32_readeflags_u64 ();
+}
+
+/* Write flags register */
+extern
Re: RFC ThreadSanitizer tests
On Thu, Dec 05, 2013 at 02:19:35PM +0400, Maxim Ostapenko wrote: Instead of mentioning the directory in the ChangeLog, mention the individual test files. ... * g++.dg/dg.exp: Prune tsan subdirectory. Thanks, I fixed the ChangeLog file. how long does it take to run make check-gcc check-g++ RUNTESTFLAGS=tsan.exp ? How much memory does it need? I've run `make check-gcc RUNTESTFLAGS=tsan.exp' (this also includes c++ tests) under `/usr/bin/time -v' and got: -Elapsed (wall clock) time (h:mm:ss or m:ss): 3:17.04 -Maximum resident set size (kbytes): 199872 kbytes The same numbers for asan are (/usr/bin/time -v make check-gcc RUNTESTFLAGS=asan.exp): -Elapsed (wall clock) time (h:mm:ss or m:ss): 3:56.38 -Maximum resident set size (kbytes): 3155264 kbytes Ok, thanks. 2013-12-05 Max Ostapenko m.ostape...@partner.samsung.com 2013-12-05 Max Ostapenko m.ostape...@partner.samsung.com actually. * c-c++-common/tsan/atomic_stack.c: New test. * c-c++-common/tsan/fd_pipe_race.c: New test. * c-c++-common/tsan/free_race.c: New test. * c-c++-common/tsan/mutexset1.c: New test. * c-c++-common/tsan/race_on_barrier.c: New test. * c-c++-common/tsan/sleep_sync.c: New test. * c-c++-common/tsan/thread_leak.c: New test. * c-c++-common/tsan/thread_leak1.c: New test. * c-c++-common/tsan/thread_leak2.c: New test. * c-c++-common/tsan/tiny_race.c: New test. * c-c++-common/tsan/tls_race.c: New test. * c-c++-common/tsan/write_in_reader_lock.c: New test. * lib/tsan-dg.exp: New file. * gcc.dg/tsan/tsan.exp: New file. * g++.dg/tsan/tsan.exp: New file. * g++.dg/dg.exp: Prune tsan subdirectory. Jakub
Re: [PATCH i386] Introduce __readeflags () and __writeeflags () intrinsics.
On Thu, Dec 5, 2013 at 11:20 AM, Kirill Yukhin kirill.yuk...@gmail.com wrote:
+ emit_insn (gen_push (gen_rtx_REG (CCmode, FLAGS_REG)));
The FLAGS_REG shuold be generated in an integer mode, appropriate for the
push!
I suppose, `word_mode' is correct mode throughout these built-ins expanding.
+ emit_insn (gen_pop (target));
+ return target;
Please note that target can be null, so you need to generate a
register and move insn in this case. Please see many examples in
i386.c expander code.
Fixed.
+case IX86_BUILTIN_WRITE_FLAGS:
+ arg0 = CALL_EXPR_ARG (exp, 0);
+ emit_insn (gen_push (expand_normal (arg0)));
This expand normal is too simple, you need to check predicate and move
argument to a mode register. Also, there are many examples througout
i386.c expanders.
Fixed using `push_operand' predicate.
It should be general_no_elim_operand to match push patterns.
+ emit_insn (gen_pop (gen_rtx_REG (CCmode, FLAGS_REG)));
Again, FLAGS_REG should be generated in a correct mode. I wonder if
flags_reg_operand correctly checks operand mode ...
Fixed.
+(define_insn *popflmode1
You can remove trailing 1 here ...
Fixed.
Bootstrapped. Tests still pass.
Changelogs untouched.
Is it ok?
OK for mainline with two predicate changes to match push/pop patterns.
Thanks,
Uros.
--
Thanks, K
gcc/config/i386/i386.c| 37 +
gcc/config/i386/i386.md | 17
gcc/config/i386/ia32intrin.h | 33 ++
gcc/testsuite/gcc.target/i386/readeflags-1.c | 40
+++
gcc/testsuite/gcc.target/i386/writeeflags-1.c | 30
5 files changed, 157 insertions(+)
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 21963bb..5bf358e 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -27909,6 +27909,10 @@ enum ix86_builtins
IX86_BUILTIN_CPU_IS,
IX86_BUILTIN_CPU_SUPPORTS,
+ /* Read/write FLAGS register built-ins. */
+ IX86_BUILTIN_READ_FLAGS,
+ IX86_BUILTIN_WRITE_FLAGS,
+
IX86_BUILTIN_MAX
};
@@ -29750,6 +29754,17 @@ ix86_init_mmx_sse_builtins (void)
UCHAR_FTYPE_UCHAR_ULONGLONG_ULONGLONG_PULONGLONG,
IX86_BUILTIN_ADDCARRYX64);
+ /* Read/write FLAGS. */
+ def_builtin (~OPTION_MASK_ISA_64BIT, __builtin_ia32_readeflags_u32,
+ UNSIGNED_FTYPE_VOID, IX86_BUILTIN_READ_FLAGS);
+ def_builtin (OPTION_MASK_ISA_64BIT, __builtin_ia32_readeflags_u64,
+ UINT64_FTYPE_VOID, IX86_BUILTIN_READ_FLAGS);
+ def_builtin (~OPTION_MASK_ISA_64BIT, __builtin_ia32_writeeflags_u32,
+ VOID_FTYPE_UNSIGNED, IX86_BUILTIN_WRITE_FLAGS);
+ def_builtin (OPTION_MASK_ISA_64BIT, __builtin_ia32_writeeflags_u64,
+ VOID_FTYPE_UINT64, IX86_BUILTIN_WRITE_FLAGS);
+
+
/* Add FMA4 multi-arg argument instructions */
for (i = 0, d = bdesc_multi_arg; i ARRAY_SIZE (bdesc_multi_arg); i++,
d++)
{
@@ -33378,6 +33393,28 @@ addcarryx:
emit_insn (gen_rtx_SET (VOIDmode, target, pat));
return target;
+case IX86_BUILTIN_READ_FLAGS:
+ emit_insn (gen_push (gen_rtx_REG (word_mode, FLAGS_REG)));
+
+ if (target == NULL_RTX
+ || !register_operand (target, word_mode)
!nonimmediate operand (...)
+ || GET_MODE (target) != word_mode)
+ target = gen_reg_rtx (word_mode);
+
+ emit_insn (gen_pop (target));
+ return target;
+
+case IX86_BUILTIN_WRITE_FLAGS:
+
+ arg0 = CALL_EXPR_ARG (exp, 0);
+ op0 = expand_normal (arg0);
+ if (!push_operand (op0, word_mode))
!general_no_elim_operand (...)
+ op0 = copy_to_mode_reg (word_mode, op0);
+
+ emit_insn (gen_push (op0));
+ emit_insn (gen_pop (gen_rtx_REG (word_mode, FLAGS_REG)));
+ return 0;
+
case IX86_BUILTIN_GATHERSIV2DF:
icode = CODE_FOR_avx2_gathersiv2df;
goto gather_gen;
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 6976124..272fbdf 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -1714,6 +1714,23 @@
pop{imodesuffix}\t%0
[(set_attr type pop)
(set_attr mode MODE)])
+
+(define_insn *pushflmode
+ [(set (match_operand:DWIH 0 push_operand =)
+ (match_operand:DWIH 1 flags_reg_operand))]
+
+ pushf{imodesuffix}
+ [(set_attr type push)
+ (set_attr mode MODE)])
+
+(define_insn *popflmode
+ [(set (match_operand:DWIH 0 flags_reg_operand)
+ (match_operand:DWIH 1 pop_operand ))]
+
+ popf{imodesuffix}
+ [(set_attr type pop)
+ (set_attr mode MODE)])
+
;; Move instructions.
diff --git a/gcc/config/i386/ia32intrin.h b/gcc/config/i386/ia32intrin.h
index b26dc46..65642e4 100644
--- a/gcc/config/i386/ia32intrin.h
+++ b/gcc/config/i386/ia32intrin.h
@@ -238,6 +238,22 @@ __rorq (unsigned long long __X, int __C)
return (__X
Re: libsanitizer merge from upstream r196489
Hi, On Thu, Dec 05, 2013 at 02:06:52PM +0400, Konstantin Serebryany wrote: Another libsanitizer merge from upstream, r196489 (Quick follow up after the r196090 merge) That commit breaks the build with: In file included from ../../../../libsanitizer/tsan/tsan_rtl_report.cc:18:0: ../../../../libsanitizer/tsan/tsan_rtl.h:29:44: fatal error: sanitizer_common/sanitizer_asm.h: No such file or directory #include sanitizer_common/sanitizer_asm.h ^ compilation terminated. Did you forgot to commit the new file? Tobias
Re: RFC ThreadSanitizer tests
On Thu, Dec 05, 2013 at 02:51:44PM +0400, Maxim Ostapenko wrote: 2013-12-05 Max Ostapenko m.ostape...@partner.samsung.com 2013-12-05 Max Ostapenkom.ostape...@partner.samsung.com actually. Ok, I'm sorry, I'll fix it. Ok to commit? Sure, that was the Ok, thanks. in the previous mail. Jakub
Re: RFC ThreadSanitizer tests
2013-12-05 Max Ostapenko m.ostape...@partner.samsung.com
2013-12-05 Max Ostapenko m.ostape...@partner.samsung.com actually.
Ok, I'm sorry, I'll fix it. Ok to commit?
-Maxim
2013-12-05 Max Ostapenko m.ostape...@partner.samsung.com
* c-c++-common/tsan/atomic_stack.c: New test.
* c-c++-common/tsan/fd_pipe_race.c: New test.
* c-c++-common/tsan/free_race.c: New test.
* c-c++-common/tsan/mutexset1.c: New test.
* c-c++-common/tsan/race_on_barrier.c: New test.
* c-c++-common/tsan/sleep_sync.c: New test.
* c-c++-common/tsan/thread_leak.c: New test.
* c-c++-common/tsan/thread_leak1.c: New test.
* c-c++-common/tsan/thread_leak2.c: New test.
* c-c++-common/tsan/tiny_race.c: New test.
* c-c++-common/tsan/tls_race.c: New test.
* c-c++-common/tsan/write_in_reader_lock.c: New test.
* lib/tsan-dg.exp: New file.
* gcc.dg/tsan/tsan.exp: New file.
* g++.dg/tsan/tsan.exp: New file.
* g++.dg/dg.exp: Prune tsan subdirectory.
diff --git a/gcc/testsuite/c-c++-common/tsan/atomic_stack.c b/gcc/testsuite/c-c++-common/tsan/atomic_stack.c
new file mode 100644
index 000..eac71b8
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/tsan/atomic_stack.c
@@ -0,0 +1,32 @@
+/* { dg-do run } */
+/* { dg-shouldfail tsan } */
+
+#include pthread.h
+#include unistd.h
+
+int Global;
+
+void *Thread1(void *x) {
+ sleep(1);
+ __atomic_fetch_add(Global, 1, __ATOMIC_RELAXED);
+ return NULL;
+}
+
+void *Thread2(void *x) {
+ Global++;
+ return NULL;
+}
+
+int main() {
+ pthread_t t[2];
+ pthread_create(t[0], NULL, Thread1, NULL);
+ pthread_create(t[1], NULL, Thread2, NULL);
+ pthread_join(t[0], NULL);
+ pthread_join(t[1], NULL);
+ return 0;
+}
+
+/* { dg-output WARNING: ThreadSanitizer: data race.*(\n|\r\n|\r) } */
+/* { dg-output Atomic write of size 4.* } */
+/* { dg-output #0 __tsan_atomic32_fetch_add.* } */
+/* { dg-output #1 Thread1.* } */
diff --git a/gcc/testsuite/c-c++-common/tsan/fd_pipe_race.c b/gcc/testsuite/c-c++-common/tsan/fd_pipe_race.c
new file mode 100644
index 000..fc76cbf
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/tsan/fd_pipe_race.c
@@ -0,0 +1,37 @@
+/* { dg-do run } */
+/* { dg-shouldfail tsan } */
+
+#include pthread.h
+#include stdio.h
+#include unistd.h
+
+int fds[2];
+
+void *Thread1(void *x) {
+ write(fds[1], a, 1);
+ return NULL;
+}
+
+void *Thread2(void *x) {
+ sleep(1);
+ close(fds[0]);
+ close(fds[1]);
+ return NULL;
+}
+
+int main() {
+ pipe(fds);
+ pthread_t t[2];
+ pthread_create(t[0], NULL, Thread1, NULL);
+ pthread_create(t[1], NULL, Thread2, NULL);
+ pthread_join(t[0], NULL);
+ pthread_join(t[1], NULL);
+}
+
+/* { dg-output WARNING: ThreadSanitizer: data race.*\n } */
+/* { dg-output Write of size 8.*\n } */
+/* { dg-output #0 close.*\n } */
+/* { dg-output #1 Thread2.*\n } */
+/* { dg-output Previous read of size 8.*\n } */
+/* { dg-output #0 write.*\n } */
+/* { dg-output #1 Thread1.*\n } */
diff --git a/gcc/testsuite/c-c++-common/tsan/free_race.c b/gcc/testsuite/c-c++-common/tsan/free_race.c
new file mode 100644
index 000..362c92b
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/tsan/free_race.c
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+/* { dg-shouldfail tsan } */
+
+#include stdlib.h
+
+void __attribute__((noinline)) foo(int *mem) {
+ free(mem);
+}
+
+void __attribute__((noinline)) bar(int *mem) {
+ mem[0] = 42;
+}
+
+int main() {
+ int *mem =(int*)malloc (100);
+ foo(mem);
+ bar(mem);
+ return 0;
+}
+
+/* { dg-output WARNING: ThreadSanitizer: heap-use-after-free.*(\n|\r\n|\r) } */
+/* { dg-output Write of size 4 at.* by main thread:(\n|\r\n|\r) } */
+/* { dg-output #0 bar.*(\n|\r\n|\r) } */
+/* { dg-output #1 main.*(\n|\r\n|\r) } */
+/* { dg-output Previous write of size 8 at.* by main thread:(\n|\r\n|\r) } */
+/* { dg-output #0 free.*(\n|\r\n|\r) } */
+/* { dg-output #\(1|2\) foo.*(\n|\r\n|\r) } */
+/* { dg-output #\(2|3\) main.*(\n|\r\n|\r) } */
diff --git a/gcc/testsuite/c-c++-common/tsan/mutexset1.c b/gcc/testsuite/c-c++-common/tsan/mutexset1.c
new file mode 100644
index 000..783f262
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/tsan/mutexset1.c
@@ -0,0 +1,41 @@
+/* { dg-do run } */
+/* { dg-shouldfail tsan } */
+
+#include pthread.h
+#include stdio.h
+#include unistd.h
+
+int Global;
+pthread_mutex_t mtx;
+
+void *Thread1(void *x) {
+ sleep(1);
+ pthread_mutex_lock(mtx);
+ Global++;
+ pthread_mutex_unlock(mtx);
+ return NULL;
+}
+
+void *Thread2(void *x) {
+ Global--;
+ return NULL;/* { dg-output .* } */
+
+}
+
+int main() {
+ pthread_mutex_init(mtx, 0);
+ pthread_t t[2];
+ pthread_create(t[0], NULL, Thread1, NULL);
+ pthread_create(t[1], NULL, Thread2, NULL);
+ pthread_join(t[0], NULL);
+ pthread_join(t[1], NULL);
+ pthread_mutex_destroy(mtx);
+ return 0;
+}
+
+/* { dg-output WARNING: ThreadSanitizer: data race.*(\n|\r\n|\r) } */
+/* { dg-output Read of
[patch][wwwdocs] gcc 4.9 changes - AMD new cores
Hello, This patch adds details about new AMD cores that got enabled in GCC-4.9. OK for the wwwdocs? Regards Ganesh cvs diff: Diffing . Index: changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.9/changes.html,v retrieving revision 1.44 diff -r1.44 changes.html 404a405,407 liSupport for new AMD family 15h processors (Excavator core) is now available through the code-march=bdver4/code and code-mtune=bdver4/code options./li
Re: libsanitizer merge from upstream r196489
clang' build is broken for me the same way Dmitry 2013/12/5 Tobias Burnus tobias.bur...@physik.fu-berlin.de: Hi, On Thu, Dec 05, 2013 at 02:06:52PM +0400, Konstantin Serebryany wrote: Another libsanitizer merge from upstream, r196489 (Quick follow up after the r196090 merge) That commit breaks the build with: In file included from ../../../../libsanitizer/tsan/tsan_rtl_report.cc:18:0: ../../../../libsanitizer/tsan/tsan_rtl.h:29:44: fatal error: sanitizer_common/sanitizer_asm.h: No such file or directory #include sanitizer_common/sanitizer_asm.h ^ compilation terminated. Did you forgot to commit the new file? Tobias
questions about COND_EXEC and SMS
hi all, *We found that COND_EXEC is better than IF_THEN_ELSE when used as expressing condition move insns, because in sched, IF_THEN_ELSE insn has a dependence on itself, and COND_EXEC has not. * Besides, IF_THEN_ELSE is not good for SMS. some backend (frv) expands condition move as IF_THEN_ELSE pattern before reload and splits IF_THEN_ELSE into COND_EXEC at split2 pass, which is a post reolad pass. However, in SMS pass(pre-reload), we can't get the accurate information of IF_THEN_ELSE insns. * However, as far as i know, COND_EXEC is not supporting in pre-reload passes. So, I'm asking for some suggestions for supporiting COND_EXEC in pre-reload passes, for me, maybe from split1 to reload pass is good enough. what work need to do for supporting that, and is there any one who has done any work on that? thanks! danxiaoqiang -- View this message in context: http://gcc.1065356.n5.nabble.com/questions-about-COND-EXEC-and-SMS-tp992591.html Sent from the gcc - patches mailing list archive at Nabble.com.
Re: libsanitizer merge from upstream r196489
On Thu, Dec 5, 2013 at 3:06 PM, Дмитрий Дьяченко dim...@gmail.com wrote: clang' build is broken for me the same way Should be fixed now (only configure/make build was affected and I tested the cmake build before committing) Dmitry 2013/12/5 Tobias Burnus tobias.bur...@physik.fu-berlin.de: Hi, On Thu, Dec 05, 2013 at 02:06:52PM +0400, Konstantin Serebryany wrote: Another libsanitizer merge from upstream, r196489 (Quick follow up after the r196090 merge) That commit breaks the build with: In file included from ../../../../libsanitizer/tsan/tsan_rtl_report.cc:18:0: ../../../../libsanitizer/tsan/tsan_rtl.h:29:44: fatal error: sanitizer_common/sanitizer_asm.h: No such file or directory #include sanitizer_common/sanitizer_asm.h ^ compilation terminated. Did you forgot to commit the new file? Looks like so, sorry. fixed by Jakub ( http://gcc.gnu.org/viewcvs?rev=205701root=gccview=rev) Tobias
Re: Silence class vs. struct warnings (opt_pass, ipa_opt_pass_d)
On Thu, Dec 5, 2013 at 11:12 AM, Oleg Endo oleg.e...@t-online.de wrote: On Thu, 2013-12-05 at 01:00 -0800, pins...@gmail.com wrote: No I don't think we want this at all. C++ is clear here. In fact we don't turn on werror for stage 1 for this exact reason. Rather it might be better to check if that flag to turn off the warning and use that. Also this warning is a bad warning for standard c++ code; clang is wrong to enable by default. Yes, warnings have to be disabled when compiling GCC, since clang complains about many more things. Anyway, these issues aside ... No I don't think we want this at all ... why is that? What's the purpose/benefit in C++ of repeatedly writing struct X* if X is already a known type? There is none, dropping those is fine (but please also look at the no longer necessary typedefs and rename structs accordingly). Richard. Cheers, Oleg
Re: Make C11 _Alignof return least not greatest alignment for a type (PR c/52023)
Joseph S. Myers jos...@codesourcery.com writes:
@@ -4994,6 +4997,22 @@ c_sizeof_or_alignof_type (location_t loc,
value = size_binop_loc (loc, CEIL_DIV_EXPR, TYPE_SIZE_UNIT (type),
size_int (TYPE_PRECISION (char_type_node)
/ BITS_PER_UNIT));
+ else if (min_alignof)
+ {
+ unsigned int align = TYPE_ALIGN (type);
+ align = MIN (align, BIGGEST_ALIGNMENT);
+#ifdef BIGGEST_FIELD_ALIGNMENT
+ align = MIN (align, BIGGEST_FIELD_ALIGNMENT);
+#endif
+ tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
+type);
+ unsigned int field_align = align;
+#ifdef ADJUST_FIELD_ALIGN
+ field_align = ADJUST_FIELD_ALIGN (field, field_align);
+#endif
+ align = MIN (align, field_align);
+ value = size_int (align / BITS_PER_UNIT);
+ }
else
value = size_int (TYPE_ALIGN_UNIT (type));
}
../../gcc/c-family/c-common.c: In function ‘tree_node*
c_sizeof_or_alignof_type(location_t, tree, bool, bool, int)’:
../../gcc/c-family/c-common.c:5007:9: error: unused variable ‘field’
[-Werror=unused-variable]
tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
^
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
And now for something completely different.
Re: [patch][wwwdocs] gcc 4.9 changes - AMD new cores
On Thu, 5 Dec 2013, Gopalasubramanian, Ganesh wrote: This patch adds details about new AMD cores that got enabled in GCC-4.9. OK for the wwwdocs? Yes, thanks. Gerald
[PATCH] Fix up passing long long in ubsan with -m32 (PR sanitizer/59333)
When we're passing ADDR_EXPR of long long int argument in -m32 mode,
the ADDR_EXPR's VAR_DECL has to have its DECL_RTL set and
corresponding stack slot allocated, it seems. Otherwise we ICE
when expanding a function with such argument. This patch does that
and adds a testcase for it.
Bootstrapped, ran ubsan testsuite on x86_64-unknown-linux-gnu.
Ok for trunk?
2013-12-05 Marek Polacek pola...@redhat.com
PR sanitizer/59333
* ubsan.c: Include rtl.h.
(ubsan_encode_value): Add new parameter. If expanding, assign
a stack slot for DECL_RTL of the temporary. Handle BOOLEAN_TYPE and
ENUMERAL_TYPE.
(ubsan_build_overflow_builtin): Adjust ubsan_encode_value
call.
* ubsan.h (ubsan_encode_value): Adjust declaration.
c-family/
* c-ubsan.c (ubsan_instrument_division): Adjust ubsan_encode_value
call.
(ubsan_instrument_shift): Likewise.
(ubsan_instrument_vla): Likewise.
testsuite/
* c-c++-common/ubsan/pr59333.c: New test.
--- gcc/c-family/c-ubsan.c.mp 2013-12-05 11:24:08.570201260 +0100
+++ gcc/c-family/c-ubsan.c 2013-12-05 11:26:25.401721067 +0100
@@ -78,8 +78,8 @@ ubsan_instrument_division (location_t lo
NULL_TREE);
data = build_fold_addr_expr_loc (loc, data);
tt = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW);
- tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0),
- ubsan_encode_value (op1));
+ tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0, false),
+ ubsan_encode_value (op1, false));
t = fold_build3 (COND_EXPR, void_type_node, t, tt, void_zero_node);
return t;
@@ -152,8 +152,8 @@ ubsan_instrument_shift (location_t loc,
t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t,
tt ? tt : integer_zero_node);
tt = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS);
- tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0),
- ubsan_encode_value (op1));
+ tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0, false),
+ ubsan_encode_value (op1, false));
t = fold_build3 (COND_EXPR, void_type_node, t, tt, void_zero_node);
return t;
@@ -174,7 +174,8 @@ ubsan_instrument_vla (location_t loc, tr
NULL_TREE);
data = build_fold_addr_expr_loc (loc, data);
tt = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_VLA_BOUND_NOT_POSITIVE);
- tt = build_call_expr_loc (loc, tt, 2, data, ubsan_encode_value (size));
+ tt = build_call_expr_loc (loc, tt, 2, data,
+ ubsan_encode_value (size, false));
t = fold_build3 (COND_EXPR, void_type_node, t, tt, void_zero_node);
return t;
--- gcc/ubsan.h.mp 2013-12-05 11:25:18.979469651 +0100
+++ gcc/ubsan.h 2013-12-05 11:25:28.958507098 +0100
@@ -41,7 +41,7 @@ extern tree ubsan_instrument_unreachable
extern tree ubsan_create_data (const char *, location_t,
const struct ubsan_mismatch_data *, ...);
extern tree ubsan_type_descriptor (tree, bool);
-extern tree ubsan_encode_value (tree);
+extern tree ubsan_encode_value (tree, bool);
extern bool is_ubsan_builtin_p (tree);
extern tree ubsan_build_overflow_builtin (tree_code, location_t, tree, tree,
tree);
--- gcc/ubsan.c.mp 2013-12-05 09:40:39.676771966 +0100
+++ gcc/ubsan.c 2013-12-05 11:35:06.907851149 +0100
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3.
#include cfgloop.h
#include ubsan.h
#include c-family/c-common.h
+#include rtl.h
/* Map from a tree to a VAR_DECL tree. */
@@ -102,45 +103,50 @@ decl_for_type_insert (tree type, tree de
/* Helper routine, which encodes a value in the pointer_sized_int_node.
Arguments with precision = POINTER_SIZE are passed directly,
- the rest is passed by reference. T is a value we are to encode. */
+ the rest is passed by reference. T is a value we are to encode.
+ IN_EXPAND_P is true if this function is called during expansion. */
tree
-ubsan_encode_value (tree t)
+ubsan_encode_value (tree t, bool in_expand_p)
{
tree type = TREE_TYPE (t);
- switch (TREE_CODE (type))
-{
-case INTEGER_TYPE:
- if (TYPE_PRECISION (type) = POINTER_SIZE)
+ const unsigned int bitsize = GET_MODE_BITSIZE (TYPE_MODE (type));
+ if (bitsize = POINTER_SIZE)
+switch (TREE_CODE (type))
+ {
+ case BOOLEAN_TYPE:
+ case ENUMERAL_TYPE:
+ case INTEGER_TYPE:
return fold_build1 (NOP_EXPR, pointer_sized_int_node, t);
+ case REAL_TYPE:
+ {
+ tree itype = build_nonstandard_integer_type (bitsize, true);
+ t = fold_build1 (VIEW_CONVERT_EXPR, itype, t);
+ return fold_convert (pointer_sized_int_node, t);
+ }
+ default:
+ gcc_unreachable ();
+ }
+ else
+{
+ if (!TREE_ADDRESSABLE (t))
+ {
+ /*
Re: RFC ThreadSanitizer tests
Ok to commit? Sure, that was the Ok, thanks. in the previous mail. Commited in r205704. -Y
Re: [PATCH] Fix up passing long long in ubsan with -m32 (PR sanitizer/59333)
On Thu, Dec 05, 2013 at 12:26:25PM +0100, Marek Polacek wrote:
--- gcc/ubsan.h.mp2013-12-05 11:25:18.979469651 +0100
+++ gcc/ubsan.h 2013-12-05 11:25:28.958507098 +0100
@@ -41,7 +41,7 @@ extern tree ubsan_instrument_unreachable
extern tree ubsan_create_data (const char *, location_t,
const struct ubsan_mismatch_data *, ...);
extern tree ubsan_type_descriptor (tree, bool);
-extern tree ubsan_encode_value (tree);
+extern tree ubsan_encode_value (tree, bool);
I'd do extern tree ubsan_encode_value (tree, bool = false);
so that we at least have some advantage from the C-C++ switch
occassionally. Then you can keep most callers of ubsan_encode_value
as is.
+ {
+ tree itype = build_nonstandard_integer_type (bitsize, true);
+ t = fold_build1 (VIEW_CONVERT_EXPR, itype, t);
+ return fold_convert (pointer_sized_int_node, t);
+ }
+ default:
+ gcc_unreachable ();
+ }
+ else
+{
+ if (!TREE_ADDRESSABLE (t))
+ {
+ /* The reason for this is that we don't want to pessimize
+ code by making vars unnecessarily addressable. */
+ tree var = create_tmp_var (type, NULL);
+ tree tem = build2 (MODIFY_EXPR, void_type_node, var, t);
+ if (in_expand_p)
+ {
+ SET_DECL_RTL (var,
+ assign_stack_temp_for_type (TYPE_MODE (type),
+ GET_MODE_SIZE (TYPE_MODE (type)), type));
Formatting looks wrong, I'd do:
rtx mem
= assign_stack_temp_for_type (TYPE_MODE (type),
GET_MODE_SIZE (TYPE_MODE (type)),
type);
SET_DECL_RTL (var, mem);
+ return build_fold_addr_expr (var);
This is something I don't understand. What will assign the value to var
aka mem then?
+ }
+ t = build_fold_addr_expr (var);
+ return build2 (COMPOUND_EXPR, TREE_TYPE (t), tem, t);
I would expect you want to use this too even if in_expand_p...
--- gcc/testsuite/c-c++-common/ubsan/pr59333.c.mp 2013-12-05
11:30:36.984759390 +0100
+++ gcc/testsuite/c-c++-common/ubsan/pr59333.c2013-12-05
11:31:36.599979040 +0100
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options -fsanitize=undefined } */
+
+long long int
+foo (long long int i, long long int j)
+{
+ return i * j;
+}
Please add a runtime testcase that verifies the values
instead. As it is long long int, you are guaranteed
it is at least 64-bit, so just make the function __attribute__((noinline,
noclone)), pass some well chosen small constants so that it overflows
and dg-output whether the library prints the correct values.
Jakub
Re: Silence class vs. struct warnings (opt_pass, ipa_opt_pass_d)
On Thu, 2013-12-05 at 12:21 +0100, Richard Biener wrote: On Thu, Dec 5, 2013 at 11:12 AM, Oleg Endo oleg.e...@t-online.de wrote: On Thu, 2013-12-05 at 01:00 -0800, pins...@gmail.com wrote: No I don't think we want this at all. C++ is clear here. In fact we don't turn on werror for stage 1 for this exact reason. Rather it might be better to check if that flag to turn off the warning and use that. Also this warning is a bad warning for standard c++ code; clang is wrong to enable by default. Yes, warnings have to be disabled when compiling GCC, since clang complains about many more things. Anyway, these issues aside ... No I don't think we want this at all ... why is that? What's the purpose/benefit in C++ of repeatedly writing struct X* if X is already a known type? There is none, dropping those is fine (but please also look at the no longer necessary typedefs There are a few typedefs: function.h: -struct ipa_opt_pass_d; -typedef struct ipa_opt_pass_d *ipa_opt_pass; +class ipa_opt_pass_d; +typedef ipa_opt_pass_d *ipa_opt_pass; cgraph.h: -typedef struct varpool_node *varpool_node_ptr; +class varpool_node; +typedef varpool_node *varpool_node_ptr; but they are used somewhere else. I could replace the uses of those typedefs in a follow up patch, but for now I wanted to keep the changes minimal. and rename structs accordingly). Sorry, I don't get it. Do you have an example in mind? Cheers, Oleg
Re: Make C11 _Alignof return least not greatest alignment for a type (PR c/52023)
On Thu, Dec 05, 2013 at 12:22:22PM +0100, Andreas Schwab wrote:
Joseph S. Myers jos...@codesourcery.com writes:
@@ -4994,6 +4997,22 @@ c_sizeof_or_alignof_type (location_t loc,
value = size_binop_loc (loc, CEIL_DIV_EXPR, TYPE_SIZE_UNIT (type),
size_int (TYPE_PRECISION (char_type_node)
/ BITS_PER_UNIT));
+ else if (min_alignof)
+ {
+ unsigned int align = TYPE_ALIGN (type);
+ align = MIN (align, BIGGEST_ALIGNMENT);
+#ifdef BIGGEST_FIELD_ALIGNMENT
+ align = MIN (align, BIGGEST_FIELD_ALIGNMENT);
+#endif
+ tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
+ type);
+ unsigned int field_align = align;
+#ifdef ADJUST_FIELD_ALIGN
+ field_align = ADJUST_FIELD_ALIGN (field, field_align);
+#endif
+ align = MIN (align, field_align);
+ value = size_int (align / BITS_PER_UNIT);
+ }
else
value = size_int (TYPE_ALIGN_UNIT (type));
}
../../gcc/c-family/c-common.c: In function ‘tree_node*
c_sizeof_or_alignof_type(location_t, tree, bool, bool, int)’:
../../gcc/c-family/c-common.c:5007:9: error: unused variable ‘field’
[-Werror=unused-variable]
tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
The following should fix it.
2013-12-05 Marek Polacek pola...@redhat.com
c-family/
* c-common.c (c_sizeof_or_alignof_type): Move a declaration into
[ADJUST_FIELD_ALIGN].
--- gcc/c-family/c-common.c.mp2 2013-12-05 12:35:38.585732312 +0100
+++ gcc/c-family/c-common.c 2013-12-05 12:36:03.351820193 +0100
@@ -5004,10 +5004,10 @@ c_sizeof_or_alignof_type (location_t loc
#ifdef BIGGEST_FIELD_ALIGNMENT
align = MIN (align, BIGGEST_FIELD_ALIGNMENT);
#endif
- tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
- type);
unsigned int field_align = align;
#ifdef ADJUST_FIELD_ALIGN
+ tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
+ type);
field_align = ADJUST_FIELD_ALIGN (field, field_align);
#endif
align = MIN (align, field_align);
Marek
Re: [GOMP4] Generation tables with omp-functions addresses for offloading.
Ping.
On 19 Nov 12:33, Michael V. Zolotukhin wrote:
Hi Jakub,
Thanks for the remarks. Updated patch is attached, and my answers are below.
This will add into the table all omp declare target functions, but you
actually want there only the outlined #pragma omp target bodies.
The question is how to find them here reliably. At least ignoring
!DECL_ARTIFICIAL (node-decl) functions would help, but still would add
e.g. cloned functions, or say #pragma omp parallel/task outlined bodies
in omp declare target functions, etc.
So, perhaps either add some extra attribute, or some flag in cgraph node,
or change create_omp_child_function_name + create_omp_child_function
+ callers that it doesn't create _omp_fn clone suffix for GOMP_target,
but say _omp_tgtfn. I think the last thing would be nice in any case.
Then you can just check if it is DECL_ARTIFICIAL with strstr (DECL_NAME
(node-decl), _omp_tgtfn) != NULL.
That's right. I added check for DECL_ARTIFICIAL for now. Renaming to
'_omp_tgtfn' could go in a separate patch I guess. Variables are handled now
as
well.
+ CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, build_fold_addr_expr
(node-decl));
As explained, the table shouldn't contain just pointers, but pairs of
pointer, size. For FUNCTION_DECLs just use size 1, we want to look up
only the first byte in them, but for var decls you want to know also
the size to register in the mapping tree.
So you need to create the structure with the two pairs, or create the
table as pointers but push two elements for each function (and VAR_DECL),
first one address, second one 1 (or size in bytes) fold_converted into
pointer type.
Yep, I fixed that using the option with table of just pointers, storing
(address, size) pairs.
You need to check if target actually supports named sections, if not, don't
create anything.
Fixed.
+ omp_finish_file ();
Only call it if (flag_openmp).
Currently that would break work with '-flto' as we don't have '-fopenmp' in
the
options list when calling lto1-compiler. I think that would be fixed soon,
when
we finish with all command-line options stuff. Also, when no symbols have
'omp
declare target' attribute, this call won't do anything except traversal
through
all symbols. What do you think, is it still worth to be guarded with
if (flag_openmp)?
Changelog:
2013-11-19 Michael Zolotukhin michael.v.zolotuk...@gmail.com
* omp-low.c: Include common/common-target.h.
(omp_finish_file): New.
* omp-low.h (omp_finish_file): Declare new function.
* toplev.c: Include omp-low.h.
(compile_file): Call omp_finish_file.
Thanks,
Michael
Jakub
---
gcc/omp-low.c | 70
+++
gcc/omp-low.h | 1 +
gcc/toplev.c | 3 +++
3 files changed, 74 insertions(+)
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 797a492..be458eb 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3. If not see
#include optabs.h
#include cfgloop.h
#include target.h
+#include common/common-target.h
#include omp-low.h
#include gimple-low.h
#include tree-cfgcleanup.h
@@ -12101,4 +12102,73 @@ make_pass_omp_simd_clone (gcc::context *ctxt)
return new pass_omp_simd_clone (ctxt);
}
+/* Create new symbol containing (address, size) pairs for omp-marked
+ functions and global variables. */
+void
+omp_finish_file (void)
+{
+ struct cgraph_node *node;
+ struct varpool_node *vnode;
+ const char *section_name = .omp_table_section;
+ tree new_decl, new_decl_type;
+ vecconstructor_elt, va_gc *v;
+ tree ctor;
+ int num = 0;
+
+ if (!targetm_common.have_named_sections)
+return;
+
+ vec_alloc (v, 0);
+
+ /* Collect all omp-target functions. */
+ FOR_EACH_DEFINED_FUNCTION (node)
+{
+ /* TODO: This check could fail on functions, created by omp
+ parallel/task pragmas. It's better to name outlined for offloading
+ functions in some different way and to check here the function name.
+ It could be something like *_omp_tgtfn in contrast with *_omp_fn
+ for functions from omp parallel/task pragmas. */
+ if (!lookup_attribute (omp declare target,
+ DECL_ATTRIBUTES (node-decl))
+ || !DECL_ARTIFICIAL (node-decl))
+ continue;
+ CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, build_fold_addr_expr
(node-decl));
+ CONSTRUCTOR_APPEND_ELT (v, NULL_TREE,
+ fold_convert (const_ptr_type_node,
+ integer_one_node));
+ num += 2;
+}
+
+ /* Collect all omp-target global variables. */
+ FOR_EACH_DEFINED_VARIABLE (vnode)
+{
+ if (!lookup_attribute (omp declare target,
+ DECL_ATTRIBUTES (vnode-decl))
+ || TREE_CODE (vnode-decl) != VAR_DECL
+
[PATCH] Improve -fsanitize=undefined a little bit
Hi!
This patch improves a little bit the generic expansion of
UBSAN_CHECK_{ADD,SUB} by either making sure that if one of the
operands is CONST_INT, it is the second one (so that the first compare
+ conditional jump is folded) or by looking at value ranges if we can prove
one of the arguments is either always negative or always non-negative.
Furthermore, the gimple_fold_stmt_to_constant_1 change earlier only
folds UBSAN_CHECK_* if both arguments are determined to be constant,
but we can actually do better than that, if value ranges prove that
there won't be an overflow, we can replace it by normal
{PLUS,MINUS,MULT}_EXPR.
Testcase I had in mind is e.g., here (in generic expansion) in fn1
we have to do 2 runtime (3 in the code) comparisons, in fn[23456]
just one, in fn7 the new tree-vrp.c code now figures out the
operation will never overflow and thus let's the optimizers generate
better code, and in fn8 we have handled it before already.
Marek, do you think you could turn this eventually into a testcases,
perhaps for different types (macroize?) and also operations (+/-/*/unary -),
with some test that just checks if things that don't overflow get the
right values and perhaps a few tests for each of the different expansions
where they do in fact overflow (those need to be one overflow per
test I'm afraid, even when you can have e.g. most of the test stuff
in some header or .c file and just tweak some macros)?
For ubsan_expand_si_overflow_neg_check, I think tree-vrp.c change can be
improved to handle also anti ranges, if the first argument of
UBSAN_CHECK_SUB has value range [0, 0] and second argument anti-range
~[x, y] where x is minimum, then it will also never overflow. Though,
I wonder if VRP doesn't canonicalize say ~[INT_MIN, INT_MIN] into
[INT_MIN+1, INT_MAX].
And, ubsan_expand_si_overflow_mul_check needs more work, first of all
to actually handle cases where we now punt (aka pretend overflow never
happens).
int
fn1 (int x, int y)
{
return x + y;
}
int
fn2 (int x)
{
return x + 7;
}
int
fn3 (unsigned char x, int y)
{
return x + y;
}
int
fn4 (int x, unsigned char y)
{
return x + y;
}
int
fn5 (unsigned char x, int y)
{
return ~x + y;
}
int
fn6 (int x, unsigned char y)
{
return x + ~y;
}
int
fn7 (unsigned char x, unsigned char y)
{
return x + y;
}
int
fn8 (void)
{
int x = 5;
int y = 6;
return x + y;
}
2013-12-05 Jakub Jelinek ja...@redhat.com
* internal-fn.c: Include stringpool.h and tree-ssanames.h.
(ubsan_expand_si_overflow_addsub_check): In the generic expansion,
try to improve generated code if one of the arguments is constant
or get_range_info says that one of the argument is always
negative or always non-negative.
* tree-vrp.c (simplify_internal_call_using_ranges): New function.
(simplify_stmt_using_ranges): Call it.
--- gcc/internal-fn.c.jj2013-12-05 09:39:36.0 +0100
+++ gcc/internal-fn.c 2013-12-05 12:24:38.925292695 +0100
@@ -34,6 +34,8 @@ along with GCC; see the file COPYING3.
#include ubsan.h
#include target.h
#include predict.h
+#include stringpool.h
+#include tree-ssanames.h
/* The names of each internal function, indexed by function number. */
const char *const internal_fn_name_array[] = {
@@ -213,28 +215,81 @@ ubsan_expand_si_overflow_addsub_check (t
if (icode == CODE_FOR_nothing)
{
rtx sub_check = gen_label_rtx ();
+ int pos_neg = 3;
/* Compute the operation. On RTL level, the addition is always
unsigned. */
res = expand_binop (mode, add_optab, op0, op1,
NULL_RTX, false, OPTAB_LIB_WIDEN);
+ /* If we can prove one of the arguments is always non-negative
+or always negative, we can do just one comparison and
+conditional jump instead of 2 at runtime, 3 present in the
+emitted code. If one of the arguments is CONST_INT, all we
+need is to make sure it is op1, then the first
+emit_cmp_and_jump_insns will be just folded. Otherwise try
+to use range info if available. */
+ if (CONST_INT_P (op0))
+ {
+ rtx tem = op0;
+ op0 = op1;
+ op1 = tem;
+ }
+ else if (CONST_INT_P (op1))
+ ;
+ else if (TREE_CODE (arg0) == SSA_NAME)
+ {
+ double_int arg0_min, arg0_max;
+ if (get_range_info (arg0, arg0_min, arg0_max) == VR_RANGE)
+ {
+ if (!arg0_min.is_negative ())
+ pos_neg = 1;
+ else if (arg0_max.is_negative ())
+ pos_neg = 2;
+ }
+ if (pos_neg != 3)
+ {
+ rtx tem = op0;
+ op0 = op1;
+ op1 = tem;
+ }
+ }
+ if (pos_neg == 3 !CONST_INT_P (op1) TREE_CODE (arg1) == SSA_NAME)
+ {
+ double_int arg1_min, arg1_max;
+ if (get_range_info (arg1, arg1_min, arg1_max) == VR_RANGE)
+ {
+ if
Re: Make C11 _Alignof return least not greatest alignment for a type (PR c/52023)
On Thu, Dec 05, 2013 at 12:43:03PM +0100, Marek Polacek wrote: The following should fix it. 2013-12-05 Marek Polacek pola...@redhat.com c-family/ * c-common.c (c_sizeof_or_alignof_type): Move a declaration into [ADJUST_FIELD_ALIGN]. Ok, thanks. Jakub
Re: [PATCH] Improve -fsanitize=undefined a little bit
On Thu, Dec 05, 2013 at 12:51:09PM +0100, Jakub Jelinek wrote:
For ubsan_expand_si_overflow_neg_check, I think tree-vrp.c change can be
improved to handle also anti ranges, if the first argument of
UBSAN_CHECK_SUB has value range [0, 0] and second argument anti-range
~[x, y] where x is minimum, then it will also never overflow. Though,
I wonder if VRP doesn't canonicalize say ~[INT_MIN, INT_MIN] into
[INT_MIN+1, INT_MAX].
That would be
int
fn9 (int x)
{
if (x == -__INT_MAX__-1)
return x;
return -x;
}
I guess, but the VR is then canonicalized properly, so we optimize
away the UBSAN_CHECK_SUB into negation. Strangely, we don't
actually optimize it, neither at gimple level or rtl level into
(int) -(unsigned) x;
Jakub
Re: [PING] [PATCH] Optional alternative base_expr in finding basis for CAND_REFs
On 12/04/13 13:08, Bill Schmidt wrote:
On Wed, 2013-12-04 at 11:26 +0100, Richard Biener wrote:
On Tue, Dec 3, 2013 at 11:04 PM, Bill Schmidt
wschm...@linux.vnet.ibm.com wrote:
On Tue, 2013-12-03 at 21:35 +0100, Richard Biener wrote:
Yufeng Zhangyufeng.zh...@arm.com wrote:
On 12/03/13 14:20, Richard Biener wrote:
On Tue, Dec 3, 2013 at 1:50 PM, Yufeng Zhangyufeng.zh...@arm.com
wrote:
On 12/03/13 06:48, Jeff Law wrote:
On 12/02/13 08:47, Yufeng Zhang wrote:
Ping~
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg03360.html
Thanks,
Yufeng
On 11/26/13 15:02, Yufeng Zhang wrote:
On 11/26/13 12:45, Richard Biener wrote:
On Thu, Nov 14, 2013 at 12:25 AM, Yufeng
Zhangyufeng.zh...@arm.com wrote:
On 11/13/13 20:54, Bill Schmidt wrote:
The second version of your original patch is ok with me with
the
following changes. Sorry for the little side adventure into
the
next-interp logic; in the end that's going to hurt more than
it
helps in
this case. Thanks for having a look at it, anyway. Thanks
also for
cleaning up this version to be less intrusive to common
interfaces; I
appreciate it.
Thanks a lot for the review. I've attached an updated patch
with the
suggested changes incorporated.
For the next-interp adventure, I was quite happy to do the
experiment; it's
a good chance of gaining insight into the pass. Many thanks
for
your prompt
replies and patience in guiding!
Everything else looks OK to me. Please ask Richard for final
approval,
as I'm not a maintainer.
First a note, I need to check on voting for Bill as the slsr
maintainer
from the steering committee. Voting was in progress just before
the
close of stage1 development so I haven't tallied the results :-)
Looking forward to some good news! :)
Yes, you are right about the non-trivial 'base' tree are rarely
shared.
The cached is introduced mainly because get_alternative_base
() may
be
called twice on the same 'base' tree, once in the
find_basis_for_candidate () for look-up and the other time in
alloc_cand_and_find_basis () for record_potential_basis (). I'm
happy
to leave out the cache if you think the benefit is trivial.
Without some sense of how expensive the lookups are vs how often
the
cache hits it's awful hard to know if the cache is worth it.
I'd say take it out unless you have some sense it's really saving
time.
It's a pretty minor implementation detail either way.
I think the affine tree routines are generally expensive; it is
worth having
a cache to avoid calling them too many times. I run the slsr-*.c
tests
under gcc.dg/tree-ssa/ and find out that the cache hit rates range
from
55.6% to 90%, with 73.5% as the average. The samples may not well
represent
the real world scenario, but they do show the fact that the 'base'
tree can
be shared to some extent. So I'd like to have the cache in the
patch.
+/* { dg-do compile } */
+/* { dg-options -O2 -fdump-tree-slsr } */
+
+typedef int arr_2[50][50];
+
+void foo (arr_2 a2, int v1)
+{
+ int i, j;
+
+ i = v1 + 5;
+ j = i;
+ a2 [i-10] [j] = 2;
+ a2 [i] [j++] = i;
+ a2 [i+20] [j++] = i;
+ a2 [i-3] [i-1] += 1;
+ return;
+}
+
+/* { dg-final { scan-tree-dump-times MEM 5 slsr } } */
+/* { dg-final { cleanup-tree-dump slsr } } */
scanning for 5 MEMs looks non-sensical. What transform do
you expect? I see other slsr testcases do similar non-sensical
checking which is bad, too.
As the slsr optimizes CAND_REF candidates by simply lowering them
to
MEM_REF from e.g. ARRAY_REF, I think scanning for the number of
MEM_REFs
is an effective check. Alternatively, I can add a follow-up
patch to
add some dumping facility in replace_ref () to print out the
replacing
actions when -fdump-tree-slsr-details is on.
I think adding some details to the dump and scanning for them would
be
better. That's the only change that is required for this to move
forward.
I've updated to patch to dump more details when
-fdump-tree-slsr-details is
on. The tests have also been updated to scan for these new dumps
instead of
MEMs.
I suggest doing it quickly. We're well past stage1 close at this
point.
The bootstrapping on x86_64 is still running. OK to commit if it
succeeds?
I still don't like it. It's using the wrong and too expensive tools
to do
stuff. What kind of bases are we ultimately interested in? Browsing
the code it looks like we're having
/* Base expression for the chain of candidates: often, but not
always, an SSA name. */
tree base_expr;
which isn't really too informative but I suppose they are all
kind-of-gimple_val()s? That said, I wonder if you can simply
use get_addr_base_and_unit_offset in place of get_alternative_base
(),
ignoring the returned offset.
'base_expr' is essentially the base address of a handled_component_p,
e.g. ARRAY_REF, COMPONENT_REF, etc. In most case, it is the address of
the object returned by get_inner_reference ().
Re: Silence class vs. struct warnings (vec)
On 12/05/2013 10:45 AM, pins...@gmail.com wrote: On Dec 5, 2013, at 1:33 AM, Oleg Endo oleg.e...@t-online.de wrote: On Thu, 2013-12-05 at 01:11 -0800, pins...@gmail.com wrote: On Dec 5, 2013, at 1:00 AM, Oleg Endo oleg.e...@t-online.de wrote: Hi, When building GCC on OSX with its native XCode/Clang tools, it outputs quite some struct X was previously declared as a class or similar warnings (-Wmismatched-tags is enabled by default). The attached patch fixes a mismatch in struct vec_prefix when referring to struct vec. Tested with make all-gcc. OK for trunk? What is this warning trying to do really? I think this is a very bad warning as points out standard code for no reason. I think the answer is here: http://llvm.org/bugs/show_bug.cgi?id=11632 Except we don't support compiling GCC with microsoft's broken compiler. So again why make this change for broken compilers that is hard to support in the first place? I'm not even sure the Microsoft compiler is broken in this regard. They have class forward declarations which change the representation of pointers to such classes, and those have to match with the definition. But I'm not sure that the C++ compiler actually errors out on class/struct mismatches (unless explicitly told to). -- Florian Weimer / Red Hat Product Security Team
Re: libsanitizer merge from upstream r196090
On Thu, Dec 5, 2013 at 1:05 AM, Jeff Law l...@redhat.com wrote: On 12/03/13 22:08, Konstantin Serebryany wrote: We need a) patches that we can review and apply to the llvm repository (w/o breaking the modern systems, of course) b) a buildbot that would run 24/7 catching regressions. If we reach a green state for a platform X and have a buildbot for it, keeping it green will require relatively small effort. Every time we break it we will notice it in minutes and fix quickly while we still have the same context fresh. Fixing old systems once in few months during merge to gcc is costly because failures accumulate. I'm well overbooked already. However, if you have x86/x86_64 systems in your build farm that can be virtualized, Unfortunately we don't. In case anyone wants to provide their build machines and maintain a bot on them here are the instructions (Evgeniy and Alexey in CC may be able to help) --kcc I can help set up a suitable VM. CentOS 5.x is old enough to trigger lots of interesting problems, but is still in widespread use. Jeff
Re: libsanitizer merge from upstream r196090
On Thu, Dec 5, 2013 at 4:22 PM, Konstantin Serebryany konstantin.s.serebry...@gmail.com wrote: On Thu, Dec 5, 2013 at 1:05 AM, Jeff Law l...@redhat.com wrote: On 12/03/13 22:08, Konstantin Serebryany wrote: We need a) patches that we can review and apply to the llvm repository (w/o breaking the modern systems, of course) b) a buildbot that would run 24/7 catching regressions. If we reach a green state for a platform X and have a buildbot for it, keeping it green will require relatively small effort. Every time we break it we will notice it in minutes and fix quickly while we still have the same context fresh. Fixing old systems once in few months during merge to gcc is costly because failures accumulate. I'm well overbooked already. However, if you have x86/x86_64 systems in your build farm that can be virtualized, Unfortunately we don't. In case anyone wants to provide their build machines and maintain a bot on them here are the instructions Actual link: http://llvm.org/docs/HowToAddABuilder.html (Evgeniy and Alexey in CC may be able to help) --kcc I can help set up a suitable VM. CentOS 5.x is old enough to trigger lots of interesting problems, but is still in widespread use. Jeff
RE: [PATCH] Strict volatile bit-fields clean-up, Take 2
Hi Richard,
I had just an idea how to solve that recursion problem without completely
ignoring the
memory mode. I hope you are gonna like it.
This time I even added a comment :-)
Ok for trunk after boot-strap and regression-testing?
Bernd.
On Tue, 3 Dec 2013 12:23:11, Richard Biener wrote:
On Tue, Dec 3, 2013 at 12:01 PM, Bernd Edlinger
bernd.edlin...@hotmail.de wrote:
Hi,
This is my proposal for ulimately getting rid of the nasty
store_fixed_bit_field recursion.
IMHO, the root of the recursion trouble is here:
@@ -1007,12 +1013,8 @@ store_fixed_bit_field (rtx op0, unsigned
if (MEM_P (op0))
{
mode = GET_MODE (op0);
if (GET_MODE_BITSIZE (mode) == 0
|| GET_MODE_BITSIZE (mode) GET_MODE_BITSIZE (word_mode))
mode = word_mode;
mode = get_best_mode (bitsize, bitnum, bitregion_start, bitregion_end,
MEM_ALIGN (op0), mode, MEM_VOLATILE_P (op0));
But, because now we always have bitregion_start and bitregion_end to limit
the access size, it is no longer necessary to restrict the largest mode, that
get_best_mode may return.
Note that this is not true, as get_bit_range itself may end up giving
up with bitregion_start = bitregion_end = 0. But maybe this is not
what you are saying here? That is, does a
gcc_assert (bitregion_start != 0 || bitregion_end != 0);
before the get_best_mode call work for you?
This patch is very similar to the previous patch, which split up the
extract_fixed_bit_field,
This time, I just split up store_fixed_bit_field and use
store_fixed_bit_field_1 to force the
strict-volatile-bitfield mode it necessary, and let get_best_mode find a
mode, that is
can be used to access the field, which is no longer impacted by the memory
context's selected
mode in this case.
I tried this patch with an ARM-Cross compiler and a large eCos application,
to see if anything
changes in the generated code with this patch, but 2 MB of code stays binary
the same,
that's a good sign.
I added the new Ada test case, and the test case from PR59134, which does no
longer re-produce
after my previous C++ memory model patch, but this fix was more or less by
chance.
Boot-Strap on X86_64-pc-linux-gnu (languages=all,ada,go) and regression-tests
still running.
Ok for trunk (when the tests succeed)?
Thanks
Bernd. 2013-12-05 Bernd Edlinger bernd.edlin...@hotmail.de
PR middle-end/59134
* expmed.c (store_fixed_bit_field_1): New function.
(store_bit_field): Use narrow_bit_field_mem and
store_fixed_bit_field_1 for -fstrict-volatile-bitfields.
(store_fixed_bit_field): Enhanced mode selection algorithm.
Call store_fixed_bit_field_1 to do the real work.
(store_fixed_bit_field_1): New function.
testsuite:
2013-12-05 Bernd Edlinger bernd.edlin...@hotmail.de
PR middle-end/59134
* gcc.c-torture/compile/pr59134.c: New test.
* gnat.dg/misaligned_volatile.adb: New test.
patch-bitfields-update-2.diff
Description: Binary data
Re: libsanitizer merge from upstream r196090
The kernel and glibc check should be done at the toplevel. So what are the minimum kernel and glibc we want to support? For us, the versions we want to support are those that have green upstream buildbots and someone to keep them green. It's hard or impossible to set a more precise combination of kernel, glibc, binutils, arch, available RAM, or other environment requirements. libsanitizer is a very platform-dependent beast, much more so than most other code in GCC or LLVM. And it's a moving target since we keep adding more platform-dependent stuff, often quite unpleasant. Today we ourselves maintain the code (i.e. have green bots) for Ubuntu 12.04, fresh Mac OS, Windows, and Android (ARM), which makes the code more or less portable to other platforms. But we have no spare cycles to support anything else ourselves. --kcc
Re: libsanitizer merge from upstream r196489
On Thu, Dec 5, 2013 at 3:18 AM, Konstantin Serebryany konstantin.s.serebry...@gmail.com wrote: On Thu, Dec 5, 2013 at 3:06 PM, Дмитрий Дьяченко dim...@gmail.com wrote: clang' build is broken for me the same way Should be fixed now (only configure/make build was affected and I tested the cmake build before committing) Dmitry 2013/12/5 Tobias Burnus tobias.bur...@physik.fu-berlin.de: Hi, On Thu, Dec 05, 2013 at 02:06:52PM +0400, Konstantin Serebryany wrote: Another libsanitizer merge from upstream, r196489 (Quick follow up after the r196090 merge) That commit breaks the build with: In file included from ../../../../libsanitizer/tsan/tsan_rtl_report.cc:18:0: ../../../../libsanitizer/tsan/tsan_rtl.h:29:44: fatal error: sanitizer_common/sanitizer_asm.h: No such file or directory #include sanitizer_common/sanitizer_asm.h ^ compilation terminated. Did you forgot to commit the new file? Looks like so, sorry. fixed by Jakub ( http://gcc.gnu.org/viewcvs?rev=205701root=gccview=rev) There are at least 2 fallouts: 1. -mx32 is broken. 2. libsanitizer now depends on libm: sanitizer_common/sanitizer_common_interceptors.inc:extern int signgam; sanitizer_common/sanitizer_common_interceptors.inc: COMMON_INTERCEPTOR_WRITE_RANGE(ctx, signgam, sizeof(signgam)); sanitizer_common/sanitizer_common_interceptors.inc: COMMON_INTERCEPTOR_WRITE_RANGE(ctx, signgam, sizeof(signgam)); sanitizer_common/sanitizer_common_interceptors.inc: COMMON_INTERCEPTOR_WRITE_RANGE(ctx, signgam, sizeof(signgam)); But -lm is added for -fsanitizer=address and I got configure:3158: /export/build/gnu/gcc-asan/build-x86_64-linux/./prev-gcc/xgcc -B/export/build/gnu/gcc-asan/build-x86_64-linux/./prev-gcc/ -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem /usr/local/x86_64-unknown-linux-gnu/include -isystem /usr/local/x86_64-unknown-linux-gnu/sys-include-o conftest -O2 -g -fsanitize=address -static-libstdc++ -static-libgcc -fsanitize=address -static-libasan -B/export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/ -B/export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/asan/ -B/export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/asan/.libs conftest.c 5 /export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/libasan.a(asan_interceptors.o): In function `__interceptor_lgamma': /export/gnu/import/git/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:2762: undefined reference to `signgam' /export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/libasan.a(asan_interceptors.o): In function `__interceptor_lgammaf': -- H.J.
Re: libsanitizer merge from upstream r196489
On Thu, Dec 5, 2013 at 4:47 AM, H.J. Lu hjl.to...@gmail.com wrote: On Thu, Dec 5, 2013 at 3:18 AM, Konstantin Serebryany konstantin.s.serebry...@gmail.com wrote: On Thu, Dec 5, 2013 at 3:06 PM, Дмитрий Дьяченко dim...@gmail.com wrote: clang' build is broken for me the same way Should be fixed now (only configure/make build was affected and I tested the cmake build before committing) Dmitry 2013/12/5 Tobias Burnus tobias.bur...@physik.fu-berlin.de: Hi, On Thu, Dec 05, 2013 at 02:06:52PM +0400, Konstantin Serebryany wrote: Another libsanitizer merge from upstream, r196489 (Quick follow up after the r196090 merge) That commit breaks the build with: In file included from ../../../../libsanitizer/tsan/tsan_rtl_report.cc:18:0: ../../../../libsanitizer/tsan/tsan_rtl.h:29:44: fatal error: sanitizer_common/sanitizer_asm.h: No such file or directory #include sanitizer_common/sanitizer_asm.h ^ compilation terminated. Did you forgot to commit the new file? Looks like so, sorry. fixed by Jakub ( http://gcc.gnu.org/viewcvs?rev=205701root=gccview=rev) There are at least 2 fallouts: 1. -mx32 is broken. 2. libsanitizer now depends on libm: sanitizer_common/sanitizer_common_interceptors.inc:extern int signgam; sanitizer_common/sanitizer_common_interceptors.inc: COMMON_INTERCEPTOR_WRITE_RANGE(ctx, signgam, sizeof(signgam)); sanitizer_common/sanitizer_common_interceptors.inc: COMMON_INTERCEPTOR_WRITE_RANGE(ctx, signgam, sizeof(signgam)); sanitizer_common/sanitizer_common_interceptors.inc: COMMON_INTERCEPTOR_WRITE_RANGE(ctx, signgam, sizeof(signgam)); But -lm is added for -fsanitizer=address and I got configure:3158: /export/build/gnu/gcc-asan/build-x86_64-linux/./prev-gcc/xgcc -B/export/build/gnu/gcc-asan/build-x86_64-linux/./prev-gcc/ -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem /usr/local/x86_64-unknown-linux-gnu/include -isystem /usr/local/x86_64-unknown-linux-gnu/sys-include-o conftest -O2 -g -fsanitize=address -static-libstdc++ -static-libgcc -fsanitize=address -static-libasan -B/export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/ -B/export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/asan/ -B/export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/asan/.libs conftest.c 5 /export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/libasan.a(asan_interceptors.o): In function `__interceptor_lgamma': /export/gnu/import/git/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:2762: undefined reference to `signgam' /export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/libasan.a(asan_interceptors.o): In function `__interceptor_lgammaf': I am testing this for the libm issue. -- H.J. -- diff --git a/libsanitizer/configure.ac b/libsanitizer/configure.ac index 7f93279..3c87984 100644 --- a/libsanitizer/configure.ac +++ b/libsanitizer/configure.ac @@ -87,7 +87,7 @@ AM_CONDITIONAL(LSAN_SUPPORTED, [test x$LSAN_SUPPORTED = xyes]) AC_CHECK_FUNCS(clock_getres clock_gettime clock_settime) # Common libraries that we need to link against for all sanitizer libs. -link_sanitizer_common='-lpthread -ldl' +link_sanitizer_common='-lpthread -ldl -lm' # Set up the set of additional libraries that we need to link against for libasan. link_libasan=$link_sanitizer_common
Re: libsanitizer merge from upstream r196489
On Thu, Dec 5, 2013 at 4:47 PM, H.J. Lu hjl.to...@gmail.com wrote: On Thu, Dec 5, 2013 at 3:18 AM, Konstantin Serebryany konstantin.s.serebry...@gmail.com wrote: On Thu, Dec 5, 2013 at 3:06 PM, Дмитрий Дьяченко dim...@gmail.com wrote: clang' build is broken for me the same way Should be fixed now (only configure/make build was affected and I tested the cmake build before committing) Dmitry 2013/12/5 Tobias Burnus tobias.bur...@physik.fu-berlin.de: Hi, On Thu, Dec 05, 2013 at 02:06:52PM +0400, Konstantin Serebryany wrote: Another libsanitizer merge from upstream, r196489 (Quick follow up after the r196090 merge) That commit breaks the build with: In file included from ../../../../libsanitizer/tsan/tsan_rtl_report.cc:18:0: ../../../../libsanitizer/tsan/tsan_rtl.h:29:44: fatal error: sanitizer_common/sanitizer_asm.h: No such file or directory #include sanitizer_common/sanitizer_asm.h ^ compilation terminated. Did you forgot to commit the new file? Looks like so, sorry. fixed by Jakub ( http://gcc.gnu.org/viewcvs?rev=205701root=gccview=rev) There are at least 2 fallouts: 1. -mx32 is broken. Please send a patch to the llvm-commits list 2. libsanitizer now depends on libm: Right... We've added -lm to clang's options recently. sanitizer_common/sanitizer_common_interceptors.inc:extern int signgam; sanitizer_common/sanitizer_common_interceptors.inc: COMMON_INTERCEPTOR_WRITE_RANGE(ctx, signgam, sizeof(signgam)); sanitizer_common/sanitizer_common_interceptors.inc: COMMON_INTERCEPTOR_WRITE_RANGE(ctx, signgam, sizeof(signgam)); sanitizer_common/sanitizer_common_interceptors.inc: COMMON_INTERCEPTOR_WRITE_RANGE(ctx, signgam, sizeof(signgam)); But -lm is added for -fsanitizer=address and I got configure:3158: /export/build/gnu/gcc-asan/build-x86_64-linux/./prev-gcc/xgcc -B/export/build/gnu/gcc-asan/build-x86_64-linux/./prev-gcc/ -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem /usr/local/x86_64-unknown-linux-gnu/include -isystem /usr/local/x86_64-unknown-linux-gnu/sys-include-o conftest -O2 -g -fsanitize=address -static-libstdc++ -static-libgcc -fsanitize=address -static-libasan -B/export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/ -B/export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/asan/ -B/export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/asan/.libs conftest.c 5 /export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/libasan.a(asan_interceptors.o): In function `__interceptor_lgamma': /export/gnu/import/git/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:2762: undefined reference to `signgam' /export/build/gnu/gcc-asan/build-x86_64-linux/prev-x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/libasan.a(asan_interceptors.o): In function `__interceptor_lgammaf': -- H.J.
Re: libsanitizer merge from upstream r196489
On Thu, Dec 05, 2013 at 04:54:23AM -0800, H.J. Lu wrote: I am testing this for the libm issue. Preapproved with proper ChangeLog entry if it works. --- a/libsanitizer/configure.ac +++ b/libsanitizer/configure.ac @@ -87,7 +87,7 @@ AM_CONDITIONAL(LSAN_SUPPORTED, [test x$LSAN_SUPPORTED = xyes]) AC_CHECK_FUNCS(clock_getres clock_gettime clock_settime) # Common libraries that we need to link against for all sanitizer libs. -link_sanitizer_common='-lpthread -ldl' +link_sanitizer_common='-lpthread -ldl -lm' # Set up the set of additional libraries that we need to link against for libasan. link_libasan=$link_sanitizer_common Jakub
Re: libsanitizer merge from upstream r196489
On Thu, Dec 5, 2013 at 4:59 AM, Konstantin Serebryany konstantin.s.serebry...@gmail.com wrote: On Thu, Dec 5, 2013 at 4:47 PM, H.J. Lu hjl.to...@gmail.com wrote: On Thu, Dec 5, 2013 at 3:18 AM, Konstantin Serebryany konstantin.s.serebry...@gmail.com wrote: On Thu, Dec 5, 2013 at 3:06 PM, Дмитрий Дьяченко dim...@gmail.com wrote: clang' build is broken for me the same way Should be fixed now (only configure/make build was affected and I tested the cmake build before committing) Dmitry 2013/12/5 Tobias Burnus tobias.bur...@physik.fu-berlin.de: Hi, On Thu, Dec 05, 2013 at 02:06:52PM +0400, Konstantin Serebryany wrote: Another libsanitizer merge from upstream, r196489 (Quick follow up after the r196090 merge) That commit breaks the build with: In file included from ../../../../libsanitizer/tsan/tsan_rtl_report.cc:18:0: ../../../../libsanitizer/tsan/tsan_rtl.h:29:44: fatal error: sanitizer_common/sanitizer_asm.h: No such file or directory #include sanitizer_common/sanitizer_asm.h ^ compilation terminated. Did you forgot to commit the new file? Looks like so, sorry. fixed by Jakub ( http://gcc.gnu.org/viewcvs?rev=205701root=gccview=rev) There are at least 2 fallouts: 1. -mx32 is broken. Please send a patch to the llvm-commits list asm/stat.h from kernel is wrong for x32. You MUST use the header files from glibc if they EXIST. It applies almost to all glibc targets. -- H.J.
Re: [PATCH] Split -fisolate-erroneous-paths into two options
On Wed, Dec 4, 2013 at 7:19 PM, Jeff Law l...@redhat.com wrote: As discussed late in this thread: http://gcc.gnu.org/ml/gcc/2013-11/msg00345.html This patch splits up the erroneous path optimization into two pieces. One which detects NULL dereferences and isolates those paths and a second which detects passing/returning a NULL pointer in cases where an attribute says a non-NULL value is required. The former is enabled by default at -O2, the latter is not enabled by default at any optimization level. Bootstrapped regression tested on x86_64-unknown-linux-gnu. Installed on the trunk. The next cleanup will be to add the warning as discussed in the same thread. Jeff * common.opt: Split up -fisolate-erroneous-paths into -fisolate-erroneous-paths-dereference and -fisolate-erroneous-paths-attribute. * invoke.texi: Corresponding changes. * gimple.c (infer_nonnull_range): Add and use new arguments to control what kind of statements can be used to infer a non-null range. * gimple.h (infer_nonnull_range): Update prototype. * tree-vrp.c (infer_value_range): Corresponding changes. * opts.c (default_options_table): Update due to option split. * gimple-ssa-isolate-paths.c: Fix trailing whitespace. (find_implicit_erroneous_behaviour): Pass additional arguments to infer_nonnull_range. (find_explicit_erroneous_behaviour): Similarly. (gate_isolate_erroneous_paths): Check both of the new options. It breaks go: /export/gnu/import/git/gcc/gcc/go/go-lang.c:276:27: error: \u2018struct gcc_options\u2019 has no member named \u2018x_flag_isolate_erroneous_paths\u2019 if (!global_options_set.x_flag_isolate_erroneous_paths) ^ /export/gnu/import/git/gcc/gcc/go/go-lang.c:277:20: error: \u2018struct gcc_options\u2019 has no member named \u2018x_flag_isolate_erroneous_paths\u2019 global_options.x_flag_isolate_erroneous_paths = 0; ^ -- H.J.
Re: [Patch, RTL] Eliminate redundant vec_select moves.
H.J. Lu wrote:
On Wed, Dec 4, 2013 at 9:29 AM, Jeff Law l...@redhat.com wrote:
On 12/04/13 09:14, H.J. Lu wrote:
+
+/* { dg-final { scan-rtl-dump deleting noop move combine { target
aarch64*-*-* } } } */
Any particular reason why it doesn't work for x86?
I don't think so. I'm pretty sure Tejas is focused on ARM platforms for the
obvious reason.
Then please add i?86-*-* x86_64-*-*.
Hi,
I tried this test on x86_64. Though the same RTL gets generated
(set (reg:Sf) (vec_select:SF (reg:V4Sf) (parallel [const 0]))
for -msse2, this optimization does not seem to trigger. Only later in a
post-reload-split does it get eliminated to something like
(set (reg:SF 21 xmm0) (reg:SF 21 xmm0))
I suspect simplify_subreg_regno () may not be returning what we want here -
sorry, I don't know enough about x86 to debug deeper.
I could either keep this test case as is or if you could give it a quick look to
see why it does not trigger, it would be useful to add x86 to this test.
Thanks,
Tejas.
Re: [PING] [PATCH] Optional alternative base_expr in finding basis for CAND_REFs
On Thu, 2013-12-05 at 12:02 +, Yufeng Zhang wrote:
On 12/04/13 13:08, Bill Schmidt wrote:
On Wed, 2013-12-04 at 11:26 +0100, Richard Biener wrote:
On Tue, Dec 3, 2013 at 11:04 PM, Bill Schmidt
wschm...@linux.vnet.ibm.com wrote:
On Tue, 2013-12-03 at 21:35 +0100, Richard Biener wrote:
Yufeng Zhangyufeng.zh...@arm.com wrote:
On 12/03/13 14:20, Richard Biener wrote:
On Tue, Dec 3, 2013 at 1:50 PM, Yufeng Zhangyufeng.zh...@arm.com
wrote:
On 12/03/13 06:48, Jeff Law wrote:
On 12/02/13 08:47, Yufeng Zhang wrote:
Ping~
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg03360.html
Thanks,
Yufeng
On 11/26/13 15:02, Yufeng Zhang wrote:
On 11/26/13 12:45, Richard Biener wrote:
On Thu, Nov 14, 2013 at 12:25 AM, Yufeng
Zhangyufeng.zh...@arm.com wrote:
On 11/13/13 20:54, Bill Schmidt wrote:
The second version of your original patch is ok with me with
the
following changes. Sorry for the little side adventure into
the
next-interp logic; in the end that's going to hurt more than
it
helps in
this case. Thanks for having a look at it, anyway. Thanks
also for
cleaning up this version to be less intrusive to common
interfaces; I
appreciate it.
Thanks a lot for the review. I've attached an updated patch
with the
suggested changes incorporated.
For the next-interp adventure, I was quite happy to do the
experiment; it's
a good chance of gaining insight into the pass. Many thanks
for
your prompt
replies and patience in guiding!
Everything else looks OK to me. Please ask Richard for final
approval,
as I'm not a maintainer.
First a note, I need to check on voting for Bill as the slsr
maintainer
from the steering committee. Voting was in progress just before
the
close of stage1 development so I haven't tallied the results :-)
Looking forward to some good news! :)
Yes, you are right about the non-trivial 'base' tree are rarely
shared.
The cached is introduced mainly because get_alternative_base
() may
be
called twice on the same 'base' tree, once in the
find_basis_for_candidate () for look-up and the other time in
alloc_cand_and_find_basis () for record_potential_basis (). I'm
happy
to leave out the cache if you think the benefit is trivial.
Without some sense of how expensive the lookups are vs how often
the
cache hits it's awful hard to know if the cache is worth it.
I'd say take it out unless you have some sense it's really saving
time.
It's a pretty minor implementation detail either way.
I think the affine tree routines are generally expensive; it is
worth having
a cache to avoid calling them too many times. I run the slsr-*.c
tests
under gcc.dg/tree-ssa/ and find out that the cache hit rates range
from
55.6% to 90%, with 73.5% as the average. The samples may not well
represent
the real world scenario, but they do show the fact that the 'base'
tree can
be shared to some extent. So I'd like to have the cache in the
patch.
+/* { dg-do compile } */
+/* { dg-options -O2 -fdump-tree-slsr } */
+
+typedef int arr_2[50][50];
+
+void foo (arr_2 a2, int v1)
+{
+ int i, j;
+
+ i = v1 + 5;
+ j = i;
+ a2 [i-10] [j] = 2;
+ a2 [i] [j++] = i;
+ a2 [i+20] [j++] = i;
+ a2 [i-3] [i-1] += 1;
+ return;
+}
+
+/* { dg-final { scan-tree-dump-times MEM 5 slsr } } */
+/* { dg-final { cleanup-tree-dump slsr } } */
scanning for 5 MEMs looks non-sensical. What transform do
you expect? I see other slsr testcases do similar non-sensical
checking which is bad, too.
As the slsr optimizes CAND_REF candidates by simply lowering them
to
MEM_REF from e.g. ARRAY_REF, I think scanning for the number of
MEM_REFs
is an effective check. Alternatively, I can add a follow-up
patch to
add some dumping facility in replace_ref () to print out the
replacing
actions when -fdump-tree-slsr-details is on.
I think adding some details to the dump and scanning for them would
be
better. That's the only change that is required for this to move
forward.
I've updated to patch to dump more details when
-fdump-tree-slsr-details is
on. The tests have also been updated to scan for these new dumps
instead of
MEMs.
I suggest doing it quickly. We're well past stage1 close at this
point.
The bootstrapping on x86_64 is still running. OK to commit if it
succeeds?
I still don't like it. It's using the wrong and too expensive tools
to do
stuff. What kind of bases are we ultimately interested in? Browsing
the code it looks like we're having
/* Base expression for the chain of candidates: often, but not
always, an SSA name. */
tree base_expr;
which isn't really too informative but I suppose they are all
kind-of-gimple_val()s? That said, I wonder if you can simply
use
Re: [Patch, RTL] Eliminate redundant vec_select moves.
On Thu, Dec 5, 2013 at 5:17 AM, Tejas Belagod tbela...@arm.com wrote:
H.J. Lu wrote:
On Wed, Dec 4, 2013 at 9:29 AM, Jeff Law l...@redhat.com wrote:
On 12/04/13 09:14, H.J. Lu wrote:
+
+/* { dg-final { scan-rtl-dump deleting noop move combine { target
aarch64*-*-* } } } */
Any particular reason why it doesn't work for x86?
I don't think so. I'm pretty sure Tejas is focused on ARM platforms for
the
obvious reason.
Then please add i?86-*-* x86_64-*-*.
Hi,
I tried this test on x86_64. Though the same RTL gets generated
(set (reg:Sf) (vec_select:SF (reg:V4Sf) (parallel [const 0]))
for -msse2, this optimization does not seem to trigger. Only later in a
post-reload-split does it get eliminated to something like
(set (reg:SF 21 xmm0) (reg:SF 21 xmm0))
I suspect simplify_subreg_regno () may not be returning what we want here -
sorry, I don't know enough about x86 to debug deeper.
Kirill, can you take a look why it doesn't work for x86?
I could either keep this test case as is or if you could give it a quick
look to see why it does not trigger, it would be useful to add x86 to this
test.
Thanks,
Tejas.
--
H.J.
Re: [PATCH] Time profiler - phase 2
Hello, there are dumps for Inkscape, it looks very well. There are few of functions that look like this (wpa cgraph): _ZL13resync_activeP19_EgeSelectOneActionii/2604322 (resync_active) @0x7f84af42cea0 Type: function definition analyzed Visibility: prevailing_def_ironly References: Referring: Read from file: libinkscape.a Availability: local First run: 4422 Function flags: executed 47x local Called by: ege_select_one_action_set_active_text/2604300 (0.34 per call) (can throw external) _ZL21commit_pending_changeP19_EgeSelectOneAction/2604327 (0.16 per call) (can throw external) _ZL34ege_select_one_action_set_propertyP8_GObjectjPK7_GValueP11_GParamSpec/2604316 (47x) (0.16 per call) (can throw external) Calls: _ZL13resync_activeP19_EgeSelectOneActionii.part.0/2604456 (10x) (0.21 per call) (can throw external) _ZL13resync_activeP19_EgeSelectOneActionii.part.0/2604456 (_ZL13resync_activeP19_EgeSelectOneActionii.part.0) @0x7f84af42cd68 Type: function definition analyzed Visibility: artificial References: _ZL7signals/2604291 (read) Referring: Read from file: libinkscape.a Availability: local First run: 0 Function flags: executed 10x local Called by: _ZL13resync_activeP19_EgeSelectOneActionii/2604322 (10x) (0.21 per call) (can throw external) First function has a profile (position is correct according to valgrind) and second not. Both of them comes from the same object file. The problem is that the second one is called according to valgrind. What does .part.X means, is it a part of function that was separated to a different function? Is there any was these two profiles could be merged? .part.X are functions created by function splitting. I assume we want to modify ipa-split.c as follows: Index: ipa-split.c === --- ipa-split.c (revision 205531) +++ ipa-split.c (working copy) @@ -1233,6 +1233,7 @@ !split_part_return_p, split_point-split_bbs, split_point-entry_bb, part); + node-tp_first_run = cur_node-tp_first_run + 1; /* For usual cloning it is enough to clear builtin only when signature changes. For partial inlining we however can not expect the part of builtin implementation to have same semantic as the whole. */ Can you, please, send me the -flto systemtaps for gimp and/or inkscape so we can decide on the patch? We should enable it earlier in stage3 rather than later. Honza
Re: [PATCH] Time profiler - phase 2
Can you, please, send me the -flto systemtaps for gimp and/or inkscape so we can decide on the patch? We should enable it earlier in stage3 rather than later. I see, the PDF was included within the tar file. Was this with -freorder-blocks-and-partition? If so, the patch is OK. I still think we should put cold parts of hot/normal function into a subsection different from unlikely section, but lets handle that incrementally. Thanks, Honza Honza
Re: [Patch, RTL] Eliminate redundant vec_select moves.
Hello, On 05 Dec 05:30, H.J. Lu wrote: Kirill, can you take a look why it doesn't work for x86? Okay, I'll look at this. -- Thanks, K
Re: Silence class vs. struct warnings (opt_pass, ipa_opt_pass_d)
On Thu, Dec 5, 2013 at 12:41 PM, Oleg Endo oleg.e...@t-online.de wrote:
On Thu, 2013-12-05 at 12:21 +0100, Richard Biener wrote:
On Thu, Dec 5, 2013 at 11:12 AM, Oleg Endo oleg.e...@t-online.de wrote:
On Thu, 2013-12-05 at 01:00 -0800, pins...@gmail.com wrote:
No I don't think we want this at all. C++ is clear here. In fact we
don't turn on werror for stage 1 for this exact reason. Rather it
might be better to check if that flag to turn off the warning and use
that. Also this warning is a bad warning for standard c++ code; clang
is wrong to enable by default.
Yes, warnings have to be disabled when compiling GCC, since clang
complains about many more things.
Anyway, these issues aside ...
No I don't think we want this at all
... why is that? What's the purpose/benefit in C++ of repeatedly
writing struct X* if X is already a known type?
There is none, dropping those is fine (but please also look at the
no longer necessary typedefs
There are a few typedefs:
function.h:
-struct ipa_opt_pass_d;
-typedef struct ipa_opt_pass_d *ipa_opt_pass;
+class ipa_opt_pass_d;
+typedef ipa_opt_pass_d *ipa_opt_pass;
cgraph.h:
-typedef struct varpool_node *varpool_node_ptr;
+class varpool_node;
+typedef varpool_node *varpool_node_ptr;
but they are used somewhere else. I could replace the uses of those
typedefs in a follow up patch, but for now I wanted to keep the changes
minimal.
I didn't mean those cerating typedefs for the pointer type.
and rename structs accordingly).
Sorry, I don't get it. Do you have an example in mind?
grep for 'typedef struct.*{' in headers. The typedef name is usually
the desired one and is used without 'struct'. So it's an orthogonal
issue.
Richard.
Cheers,
Oleg
Re: [PING] [PATCH] Optional alternative base_expr in finding basis for CAND_REFs
On 12/05/13 13:21, Bill Schmidt wrote:
On Thu, 2013-12-05 at 12:02 +, Yufeng Zhang wrote:
On 12/04/13 13:08, Bill Schmidt wrote:
On Wed, 2013-12-04 at 11:26 +0100, Richard Biener wrote:
[snip]
I'm not sure what you're suggesting that he use get_inner_reference on
at this point. At the point where the affine machinery is invoked, the
memory reference was already expanded with get_inner_reference, and
there was no basis involving the SSA name produced as the base. The
affine machinery is invoked on that SSA name to see if it is hiding
another base. There's no additional memory reference to use
get_inner_reference on, just potentially some pointer arithmetic.
That said, if we have real compile-time issues, we should hold off on
this patch for this release.
Yufeng, please time some reasonably large benchmarks (some version of
SPECint or similar) and report back here before the patch goes in.
I've got some build time data for SPEC2Kint.
On x86_64 the -O3 builds take about 4m7.5s with or without the patch
(consistent over 3 samples). The difference of the -O3 build time on
arm cortex-a15 is also within 2 seconds.
The bootstrapping time on x86_64 is 134m48.040s without the patch and
134m46.889s with the patch; this data is preliminary as I only sampled
once, but the difference of the bootstrapping time on arm cortex-a15 is
also within 5 seconds.
I can further time SPEC2006int if necessary.
I've also prepared a patch to further reduce the number of calls to
tree-affine expansion, by checking whether or not the passed-in BASE in
get_alternative_base () is simply an SSA_NAME of a declared variable.
Please see the inlined patch below.
Thanks,
Yufeng
diff --git a/gcc/gimple-ssa-strength-reduction.c
b/gcc/gimple-ssa-strength-reduction.c
index 26502c3..2984f06 100644
--- a/gcc/gimple-ssa-strength-reduction.c
+++ b/gcc/gimple-ssa-strength-reduction.c
@@ -437,13 +437,22 @@ get_alternative_base (tree base)
if (result == NULL)
{
- tree expr;
- aff_tree aff;
+ tree expr = NULL;
+ gimple def = NULL;
- tree_to_aff_combination_expand (base, TREE_TYPE (base),
-aff,name_expansions);
- aff.offset = tree_to_double_int (integer_zero_node);
- expr = aff_combination_to_tree (aff);
+ if (TREE_CODE (base) == SSA_NAME)
+ def = SSA_NAME_DEF_STMT (base);
+
+ /* Avoid calling expensive tree-affine expansion if BASE
+ is just an SSA_NAME of, e.g. a para_decl. */
+ if (!def || (is_gimple_assign (def) gimple_assign_lhs (def) ==
base))
Well, that just isn't right. !def indicates you have a parameter, so
why call tree_to_aff_combination_expand in that case? Just forget this
addition and check for flag_expensive_optimizations as Richard suggested
in another branch of this thread.
I thought every SSA_NAME has its DEF_STMT, at least in the cases which I
checked they are GIMPLE_NOPs; that's why I used !def to check for cases
where BASE is not an SSA_NAME (bad programming habit I guess).
Anyway, I'll leave out this addition.
Previous version of the patch is ok with this change, and with a comment
added that we should eliminate this backtracking with better forward
analysis in a future release.
Thanks. The following inlined diff is the incremental change.
Thanks again for your review and help.
Regards,
Yufeng
diff --git a/gcc/gimple-ssa-strength-reduction.c
b/gcc/gimple-ssa-strength-reduction.c
index 26502c3..f406794 100644
--- a/gcc/gimple-ssa-strength-reduction.c
+++ b/gcc/gimple-ssa-strength-reduction.c
@@ -428,7 +428,10 @@ static struct pointer_map_t *alt_base_map;
/* Given BASE, use the tree affine combiniation facilities to
find the underlying tree expression for BASE, with any
- immediate offset excluded. */
+ immediate offset excluded.
+
+ N.B. we should eliminate this backtracking with better forward
+ analysis in a future release. */
static tree
get_alternative_base (tree base)
@@ -556,7 +559,7 @@ find_basis_for_candidate (slsr_cand_t c)
}
}
- if (!basis c-kind == CAND_REF)
+ if (flag_expensive_optimizations !basis c-kind == CAND_REF)
{
tree alt_base_expr = get_alternative_base (c-base_expr);
if (alt_base_expr)
@@ -641,7 +644,7 @@ alloc_cand_and_find_basis (enum cand_kind kind,
gimple gs, tree base,
c-basis = find_basis_for_candidate (c);
record_potential_basis (c, base);
- if (kind == CAND_REF)
+ if (flag_expensive_optimizations kind == CAND_REF)
{
tree alt_base = get_alternative_base (base);
if (alt_base)
Re: [PATCH] Strict volatile bit-fields clean-up, Take 2
On Thu, Dec 5, 2013 at 1:27 PM, Bernd Edlinger
bernd.edlin...@hotmail.de wrote:
Hi Richard,
I had just an idea how to solve that recursion problem without completely
ignoring the
memory mode. I hope you are gonna like it.
This time I even added a comment :-)
Ehm, ...
+ /* If MODE has no size i.e. BLKmode or is larger than WORD_MODE
+or BITREGION_END is used we can use WORD_MODE as upper limit.
+However, if the field is too unaligned to be fetched with a single
+access, we also have to use WORD_MODE. This can happen in Ada. */
if (GET_MODE_BITSIZE (mode) == 0
- || GET_MODE_BITSIZE (mode) GET_MODE_BITSIZE (word_mode))
+ || GET_MODE_BITSIZE (mode) GET_MODE_BITSIZE (word_mode)
+ || bitregion_end != 0
+ || bitnum % BITS_PER_UNIT + bitsize GET_MODE_SIZE (mode)
+ || (STRICT_ALIGNMENT
+ bitnum % GET_MODE_ALIGNMENT (mode) + bitsize
+ GET_MODE_BITSIZE (mode)))
mode = word_mode;
If the field is unaligned how does choosing a larger mode help? We should
always be able to use multiple accesses with a smaller mode in this case.
So - I'd buy the || bitregion_end != 0 thing but the rest ...? So, ok if
that alone fixes the bug. Note that when bitregion_end == 0 the
access should be limited by the mode we pass to get_best_mode.
Btw, it should be always possible to return QImode here, so I wonder
how enlarging the mode fixes the underlying issue (maybe the bug
is really elsewhere?)
Thanks,
Richard.
Ok for trunk after boot-strap and regression-testing?
Bernd.
On Tue, 3 Dec 2013 12:23:11, Richard Biener wrote:
On Tue, Dec 3, 2013 at 12:01 PM, Bernd Edlinger
bernd.edlin...@hotmail.de wrote:
Hi,
This is my proposal for ulimately getting rid of the nasty
store_fixed_bit_field recursion.
IMHO, the root of the recursion trouble is here:
@@ -1007,12 +1013,8 @@ store_fixed_bit_field (rtx op0, unsigned
if (MEM_P (op0))
{
mode = GET_MODE (op0);
if (GET_MODE_BITSIZE (mode) == 0
|| GET_MODE_BITSIZE (mode) GET_MODE_BITSIZE (word_mode))
mode = word_mode;
mode = get_best_mode (bitsize, bitnum, bitregion_start, bitregion_end,
MEM_ALIGN (op0), mode, MEM_VOLATILE_P (op0));
But, because now we always have bitregion_start and bitregion_end to limit
the access size, it is no longer necessary to restrict the largest mode,
that
get_best_mode may return.
Note that this is not true, as get_bit_range itself may end up giving
up with bitregion_start = bitregion_end = 0. But maybe this is not
what you are saying here? That is, does a
gcc_assert (bitregion_start != 0 || bitregion_end != 0);
before the get_best_mode call work for you?
This patch is very similar to the previous patch, which split up the
extract_fixed_bit_field,
This time, I just split up store_fixed_bit_field and use
store_fixed_bit_field_1 to force the
strict-volatile-bitfield mode it necessary, and let get_best_mode find a
mode, that is
can be used to access the field, which is no longer impacted by the memory
context's selected
mode in this case.
I tried this patch with an ARM-Cross compiler and a large eCos application,
to see if anything
changes in the generated code with this patch, but 2 MB of code stays
binary the same,
that's a good sign.
I added the new Ada test case, and the test case from PR59134, which does
no longer re-produce
after my previous C++ memory model patch, but this fix was more or less
by chance.
Boot-Strap on X86_64-pc-linux-gnu (languages=all,ada,go) and
regression-tests
still running.
Ok for trunk (when the tests succeed)?
Thanks
Bernd.
Re: Silence class vs. struct warnings (opt_pass, ipa_opt_pass_d)
On Thu, 2013-12-05 at 14:56 +0100, Richard Biener wrote:
but they are used somewhere else. I could replace the uses of those
typedefs in a follow up patch, but for now I wanted to keep the changes
minimal.
I didn't mean those cerating typedefs for the pointer type.
and rename structs accordingly).
Sorry, I don't get it. Do you have an example in mind?
grep for 'typedef struct.*{' in headers. The typedef name is usually
the desired one and is used without 'struct'. So it's an orthogonal
issue.
Ah, do you mean converting this stuff ...
typedef struct
{
cgraph_node_set set;
unsigned index;
} cgraph_node_set_iterator;
... to ...
struct cgraph_node_set_iterator
{
right?
Sure, no problem. But I'd rather do it step by step in separate
patches. Is it OK to apply the following two as a start?
http://gcc.gnu.org/ml/gcc-patches/2013-12/msg00458.html
http://gcc.gnu.org/ml/gcc-patches/2013-12/msg00460.html
Cheers,
Oleg
PATCH: Fix libsanitizer for x32
On Thu, Dec 5, 2013 at 4:59 AM, Konstantin Serebryany
konstantin.s.serebry...@gmail.com wrote:
On Thu, Dec 5, 2013 at 4:47 PM, H.J. Lu hjl.to...@gmail.com wrote:
There are at least 2 fallouts:
1. -mx32 is broken.
Please send a patch to the llvm-commits list
I am enclosing 2 patches here. You can test x32 on Ubuntu
13.04 or newer.
struct stat defined in asm/stat.h is incorrect for x32. asm/stat.h
is included to get struct __old_kernel_stat. But struct __old_kernel_stat
isn't used for x86-64 and x32. The first patch includes sys/stat.h instead
of asm/stat.h and comments out size check of struct __old_kernel_stat
for x86-64.
Some fields in shmid_ds as well as clock_t are int64 for x32. The second
patch corrects them for x32.
Thanks.
--
H.J.
From aa91c65dd8ef7f2fcccae0ca164bfa547e169a5c Mon Sep 17 00:00:00 2001
From: H.J. Lu hjl.to...@gmail.com
Date: Thu, 5 Dec 2013 06:19:31 -0800
Subject: [PATCH 1/2] Include sys/stat.h for x86-64
struct stat defined in asm/stat.h is incorrect for x32. asm/stat.h
is included to get struct __old_kernel_stat. But struct __old_kernel_stat
isn't used for x86-64 and x32. This patch includes sys/stat.h instead
of asm/stat.h and comments out size check of struct __old_kernel_stat
for x86-64.
---
libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc
b/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc
index 01de9c9..bc37df0 100644
--- a/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc
+++ b/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc
@@ -27,6 +27,9 @@
// are not defined anywhere in userspace headers. Fake them. This seems to work
// fine with newer headers, too.
#include asm/posix_types.h
+#if defined(__x86_64__)
+#include sys/stat.h
+#else
#define ino_t __kernel_ino_t
#define mode_t __kernel_mode_t
#define nlink_t __kernel_nlink_t
@@ -41,6 +44,7 @@
#undef uid_t
#undef gid_t
#undef off_t
+#endif
#include linux/aio_abi.h
@@ -58,7 +62,7 @@ namespace __sanitizer {
unsigned struct_statfs64_sz = sizeof(struct statfs64);
} // namespace __sanitizer
-#if !defined(__powerpc64__)
+#if !defined(__powerpc64__) !defined(__x86_64__)
COMPILER_CHECK(struct___old_kernel_stat_sz == sizeof(struct
__old_kernel_stat));
#endif
--
1.8.3.1
From a8d2227ae1ce9da963dca9e3e1e8e04c3db7ad46 Mon Sep 17 00:00:00 2001
From: H.J. Lu hjl.to...@gmail.com
Date: Thu, 5 Dec 2013 06:24:36 -0800
Subject: [PATCH 2/2] Correct __sanitizer_shmid_ds/__sanitizer_clock_t for x32
Some fields in shmid_ds as well as clock_t are int64 for x32. This
patch corrects them for x32.
---
.../sanitizer_common/sanitizer_platform_limits_posix.h | 16
1 file changed, 16 insertions(+)
diff --git a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
index f98ebea..be6e6cf 100644
--- a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
+++ b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
@@ -167,6 +167,11 @@ namespace __sanitizer {
#elif !defined(__powerpc64__)
uptr __unused0;
#endif
+ #if defined(__x86_64__) !defined(_LP64)
+u64 shm_atime;
+u64 shm_dtime;
+u64 shm_ctime;
+ #else
uptr shm_atime;
#ifndef _LP64
uptr __unused1;
@@ -179,14 +184,21 @@ namespace __sanitizer {
#ifndef _LP64
uptr __unused3;
#endif
+ #endif
#ifdef __powerpc__
uptr shm_segsz;
#endif
int shm_cpid;
int shm_lpid;
+ #if defined(__x86_64__) !defined(_LP64)
+u64 shm_nattch;
+u64 __unused4;
+u64 __unused5;
+ #else
uptr shm_nattch;
uptr __unused4;
uptr __unused5;
+ #endif
};
#endif // SANITIZER_LINUX !SANITIZER_ANDROID
@@ -294,7 +306,11 @@ namespace __sanitizer {
};
#endif
+#if defined(__x86_64__) !defined(_LP64)
+ typedef long long __sanitizer_clock_t;
+#else
typedef long __sanitizer_clock_t;
+#endif
#if SANITIZER_LINUX
#if defined(_LP64) || defined(__x86_64__) || defined(__powerpc__)
--
1.8.3.1
Re: libsanitizer merge from upstream r196489
On Thu, Dec 5, 2013 at 5:00 AM, Jakub Jelinek ja...@redhat.com wrote: On Thu, Dec 05, 2013 at 04:54:23AM -0800, H.J. Lu wrote: I am testing this for the libm issue. Preapproved with proper ChangeLog entry if it works. --- a/libsanitizer/configure.ac +++ b/libsanitizer/configure.ac @@ -87,7 +87,7 @@ AM_CONDITIONAL(LSAN_SUPPORTED, [test x$LSAN_SUPPORTED = xyes]) AC_CHECK_FUNCS(clock_getres clock_gettime clock_settime) # Common libraries that we need to link against for all sanitizer libs. -link_sanitizer_common='-lpthread -ldl' +link_sanitizer_common='-lpthread -ldl -lm' # Set up the set of additional libraries that we need to link against for libasan. link_libasan=$link_sanitizer_common This is the patch I checked in. -- H.J. --- diff --git a/libsanitizer/ChangeLog b/libsanitizer/ChangeLog index 0af33d0..5d02ec3 100644 --- a/libsanitizer/ChangeLog +++ b/libsanitizer/ChangeLog @@ -1,3 +1,8 @@ +2013-12-05 H.J. Lu hongjiu...@intel.com + +* configure.ac (link_sanitizer_common): Add -lm. +* configure: Regenerated. + 2013-12-05 Kostya Serebryany k...@google.com * All source files: Merge from upstream r196489. diff --git a/libsanitizer/configure b/libsanitizer/configure index e5c3206..6db2a1f 100755 --- a/libsanitizer/configure +++ b/libsanitizer/configure @@ -14564,7 +14564,7 @@ done # Common libraries that we need to link against for all sanitizer libs. -link_sanitizer_common='-lpthread -ldl' +link_sanitizer_common='-lpthread -ldl -lm' # Set up the set of additional libraries that we need to link against for libasan. link_libasan=$link_sanitizer_common diff --git a/libsanitizer/configure.ac b/libsanitizer/configure.ac index 7f93279..3c87984 100644 --- a/libsanitizer/configure.ac +++ b/libsanitizer/configure.ac @@ -87,7 +87,7 @@ AM_CONDITIONAL(LSAN_SUPPORTED, [test x$LSAN_SUPPORTED = xyes]) AC_CHECK_FUNCS(clock_getres clock_gettime clock_settime) # Common libraries that we need to link against for all sanitizer libs. -link_sanitizer_common='-lpthread -ldl' +link_sanitizer_common='-lpthread -ldl -lm' # Set up the set of additional libraries that we need to link against for libasan. link_libasan=$link_sanitizer_common
[gomp4] splay tree implementation for future OpenACC runtime library usage.
Hi!
Here is a patch that changes the splay tree implementation. Specifically,
generalizes the implementation so that it can be used by other functions
outside of its use by the functions within target.c.
Would appreciate review of the changes.
Thanks!
Generalize splay tree implementation for future OpenACC Runtime
Library usage.
* Makefile.am (libgomp_la_SOURCES): Add splay-tree.c.
* Makefile.in: Regenerate.
* target.c: Move definition of splay_tree_key_s to target.h
* target.h: New file with definition from target.c.
* splay-tree.h: (rotate_left, rotate_right, splay_tree_splay,
splay_tree_insert, splay_tree_remove, splay_tree_lookup):
Move functions to splay-tree.c.
(splay_tree_lookup, splay_tree_insert, splay_tree_remove,
splay_compare): New declarations.
* splay-tree.c: New file with functions from splay-tree.h.
Index: splay-tree.c
===
--- splay-tree.c(revision 0)
+++ splay-tree.c(revision 0)
@@ -0,0 +1,227 @@
+/* A splay-tree datatype.
+ Copyright 1998-2013
+ Free Software Foundation, Inc.
+ Contributed by Mark Mitchell (m...@markmitchell.com).
+
+ This file is part of the GNU OpenMP Library (libgomp).
+
+ Libgomp is free software; you can redistribute it and/or modify it
+ under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3, or (at your option)
+ any later version.
+
+ Libgomp is distributed in the hope that it will be useful, but WITHOUT ANY
+ WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ more details.
+
+ Under Section 7 of GPL version 3, you are granted additional
+ permissions described in the GCC Runtime Library Exception, version
+ 3.1, as published by the Free Software Foundation.
+
+ You should have received a copy of the GNU General Public License and
+ a copy of the GCC Runtime Library Exception along with this program;
+ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+ http://www.gnu.org/licenses/. */
+
+/* The splay tree code copied from include/splay-tree.h and adjusted,
+ so that all the data lives directly in splay_tree_node_s structure
+ and no extra allocations are needed.
+
+ Files including this header should before including it add:
+typedef struct splay_tree_node_s *splay_tree_node;
+typedef struct splay_tree_s *splay_tree;
+typedef struct splay_tree_key_s *splay_tree_key;
+ define splay_tree_key_s structure, and define
+ splay_compare inline function. */
+
+/* For an easily readable description of splay-trees, see:
+
+ Lewis, Harry R. and Denenberg, Larry. Data Structures and Their
+ Algorithms. Harper-Collins, Inc. 1991.
+
+ The major feature of splay trees is that all basic tree operations
+ are amortized O(log n) time for a tree with n nodes. */
+
+/* Forward declaration for a node in the tree. */
+typedef struct splay_tree_node_s *splay_tree_node;
+typedef struct splay_tree_s *splay_tree;
+typedef struct splay_tree_key_s *splay_tree_key;
+
+#include libgomp.h
+#include splay-tree.h
+
+/* Rotate the edge joining the left child N with its parent P. PP is the
+ grandparents' pointer to P. */
+
+static inline void
+rotate_left (splay_tree_node *pp, splay_tree_node p, splay_tree_node n)
+{
+ splay_tree_node tmp;
+ tmp = n-right;
+ n-right = p;
+ p-left = tmp;
+ *pp = n;
+}
+
+/* Rotate the edge joining the right child N with its parent P. PP is the
+ grandparents' pointer to P. */
+
+static inline void
+rotate_right (splay_tree_node *pp, splay_tree_node p, splay_tree_node n)
+{
+ splay_tree_node tmp;
+ tmp = n-left;
+ n-left = p;
+ p-right = tmp;
+ *pp = n;
+}
+
+/* Bottom up splay of KEY. */
+
+static void
+splay_tree_splay (splay_tree sp, splay_tree_key key)
+{
+ if (sp-root == NULL)
+return;
+
+ do {
+int cmp1, cmp2;
+splay_tree_node n, c;
+
+n = sp-root;
+cmp1 = splay_compare (key, n-key);
+
+/* Found. */
+if (cmp1 == 0)
+ return;
+
+/* Left or right? If no child, then we're done. */
+if (cmp1 0)
+ c = n-left;
+else
+ c = n-right;
+if (!c)
+ return;
+
+/* Next one left or right? If found or no child, we're done
+ after one rotation. */
+cmp2 = splay_compare (key, c-key);
+if (cmp2 == 0
+ || (cmp2 0 !c-left)
+ || (cmp2 0 !c-right))
+ {
+ if (cmp1 0)
+ rotate_left (sp-root, n, c);
+ else
+ rotate_right (sp-root, n, c);
+ return;
+ }
+
+/* Now we have the four cases of double-rotation. */
+if (cmp1 0 cmp2 0)
+ {
+ rotate_left (n-left, c, c-left);
+ rotate_left (sp-root, n, n-left);
+ }
+else if (cmp1 0 cmp2 0)
+ {
+
[PATCH] Fix PR59058
This finally fixes PR59058.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Richard.
2013-12-05 Richard Biener rguent...@suse.de
PR tree-optimization/59058
* tree-vectorizer.h (struct _loop_vec_info): Add num_itersm1
member.
(LOOP_VINFO_NITERSM1): New macro.
* tree-vect-loop-manip.c (slpeel_tree_peel_loop_to_edge): Express
the vector loop entry test in terms of scalar latch executions.
(vect_do_peeling_for_alignment): Update LOOP_VINFO_NITERSM1.
* tree-vect-loop.c (vect_get_loop_niters): Also return the
number of latch executions.
(new_loop_vec_info): Initialize LOOP_VINFO_NITERSM1.
(vect_analyze_loop_form): Likewise.
(vect_generate_tmps_on_preheader): Compute the number of
vectorized iterations differently.
* gcc.dg/torture/pr59058.c: New testcase.
Index: trunk/gcc/testsuite/gcc.dg/torture/pr59058.c
===
*** /dev/null 1970-01-01 00:00:00.0 +
--- trunk/gcc/testsuite/gcc.dg/torture/pr59058.c2013-12-05
14:51:54.231300709 +0100
***
*** 0
--- 1,19
+ /* { dg-do run } */
+
+ extern void abort (void);
+
+ short b = 0;
+
+ int
+ main ()
+ {
+ int c = 0;
+ l1:
+ b++;
+ c |= b;
+ if (b)
+ goto l1;
+ if (c != -1)
+ abort ();
+ return 0;
+ }
Index: trunk/gcc/tree-vect-loop-manip.c
===
*** trunk.orig/gcc/tree-vect-loop-manip.c 2013-12-05 12:27:23.0
+0100
--- trunk/gcc/tree-vect-loop-manip.c2013-12-05 14:50:59.468666726 +0100
*** slpeel_tree_peel_loop_to_edge (struct lo
*** 1061,1067
gimple_stmt_iterator gsi;
edge exit_e = single_exit (loop);
source_location loop_loc;
- tree cost_pre_condition = NULL_TREE;
/* There are many aspects to how likely the first loop is going to be
executed.
Without histogram we can't really do good job. Simply set it to
2/3, so the first loop is not reordered to the end of function and
--- 1061,1066
*** slpeel_tree_peel_loop_to_edge (struct lo
*** 1263,1283
/* Epilogue peeling. */
if (!update_first_loop_count)
{
pre_condition =
! fold_build2 (LE_EXPR, boolean_type_node, *first_niters,
!build_int_cst (TREE_TYPE (*first_niters), 0));
! if (check_profitability)
! {
! tree scalar_loop_iters
! = unshare_expr (LOOP_VINFO_NITERS_UNCHANGED
! (loop_vec_info_for_loop (loop)));
! cost_pre_condition =
! fold_build2 (LE_EXPR, boolean_type_node, scalar_loop_iters,
!build_int_cst (TREE_TYPE (scalar_loop_iters), th));
!
! pre_condition = fold_build2 (TRUTH_OR_EXPR, boolean_type_node,
! cost_pre_condition, pre_condition);
! }
if (cond_expr)
{
pre_condition =
--- 1262,1278
/* Epilogue peeling. */
if (!update_first_loop_count)
{
+ loop_vec_info loop_vinfo = loop_vec_info_for_loop (loop);
+ tree scalar_loop_iters = LOOP_VINFO_NITERSM1 (loop_vinfo);
+ unsigned limit = LOOP_VINFO_VECT_FACTOR (loop_vinfo) - 1;
+ if (LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo))
+ limit = limit + 1;
+ if (check_profitability
+ th limit)
+ limit = th;
pre_condition =
! fold_build2 (LT_EXPR, boolean_type_node, scalar_loop_iters,
!build_int_cst (TREE_TYPE (scalar_loop_iters), limit));
if (cond_expr)
{
pre_condition =
*** vect_do_peeling_for_alignment (loop_vec_
*** 1922,1927
--- 1917,1925
/* Update number of times loop executes. */
LOOP_VINFO_NITERS (loop_vinfo) = fold_build2 (MINUS_EXPR,
TREE_TYPE (ni_name), ni_name, niters_of_prolog_loop);
+ LOOP_VINFO_NITERSM1 (loop_vinfo) = fold_build2 (MINUS_EXPR,
+ TREE_TYPE (ni_name),
+ LOOP_VINFO_NITERSM1 (loop_vinfo), niters_of_prolog_loop);
if (types_compatible_p (sizetype, TREE_TYPE (niters_of_prolog_loop)))
wide_prolog_niters = niters_of_prolog_loop;
Index: trunk/gcc/tree-vect-loop.c
===
*** trunk.orig/gcc/tree-vect-loop.c 2013-12-05 12:27:23.0 +0100
--- trunk/gcc/tree-vect-loop.c 2013-12-05 14:36:14.917389587 +0100
*** vect_analyze_scalar_cycles (loop_vec_inf
*** 791,802
/* Function vect_get_loop_niters.
Determine how many iterations the loop is executed and place it
!in NUMBER_OF_ITERATIONS.
Return the loop exit condition. */
static gimple
! vect_get_loop_niters (struct loop *loop, tree *number_of_iterations)
{
tree niters;
--- 791,804
/* Function vect_get_loop_niters.
Determine how
[Patch,avr]: Fix wrong warning PR59396
This is a fix of a wrong warning for a bas ISR name. The assumption was that if DECL_ASSEMBLER_NAME is set, it would always starts with a *. This is not the case for LTO compiler where the assembler name is the plain name of the function (except an assembler name is set). Thus, do a more restrictive test if the first character of the function name has to be skipped. Ok to commit? Johann PR target/59396 * config/avr/avr.c (avr_set_current_function): If the first char of the function name is skipped, make sure it is actually '*'. Index: config/avr/avr.c === --- config/avr/avr.c (revision 205709) +++ config/avr/avr.c (working copy) @@ -599,7 +599,8 @@ avr_set_current_function (tree decl) tree ret = TREE_TYPE (TREE_TYPE (decl)); const char *name; - name = DECL_ASSEMBLER_NAME_SET_P (decl) + name = (DECL_ASSEMBLER_NAME_SET_P (decl) + '*' == *IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl))) /* Remove the leading '*' added in set_user_assembler_name. */ ? 1 + IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)) : IDENTIFIER_POINTER (DECL_NAME (decl));
Re: [Patch,avr]: Fix wrong warning PR59396
On Thu, Dec 5, 2013 at 3:53 PM, Georg-Johann Lay a...@gjlay.de wrote: This is a fix of a wrong warning for a bas ISR name. The assumption was that if DECL_ASSEMBLER_NAME is set, it would always starts with a *. This is not the case for LTO compiler where the assembler name is the plain name of the function (except an assembler name is set). That sounds odd to me. Does the bug reproduce with -fwhole-program? Or if the interrupt handler is static? Richard. Thus, do a more restrictive test if the first character of the function name has to be skipped. Ok to commit? Johann PR target/59396 * config/avr/avr.c (avr_set_current_function): If the first char of the function name is skipped, make sure it is actually '*'.
Re: [PATCH] Fix up passing long long in ubsan with -m32 (PR sanitizer/59333)
On Thu, Dec 05, 2013 at 12:37:27PM +0100, Jakub Jelinek wrote:
On Thu, Dec 05, 2013 at 12:26:25PM +0100, Marek Polacek wrote:
--- gcc/ubsan.h.mp 2013-12-05 11:25:18.979469651 +0100
+++ gcc/ubsan.h 2013-12-05 11:25:28.958507098 +0100
@@ -41,7 +41,7 @@ extern tree ubsan_instrument_unreachable
extern tree ubsan_create_data (const char *, location_t,
const struct ubsan_mismatch_data *, ...);
extern tree ubsan_type_descriptor (tree, bool);
-extern tree ubsan_encode_value (tree);
+extern tree ubsan_encode_value (tree, bool);
I'd do extern tree ubsan_encode_value (tree, bool = false);
so that we at least have some advantage from the C-C++ switch
occassionally. Then you can keep most callers of ubsan_encode_value
as is.
Cool, done.
+ {
+ tree itype = build_nonstandard_integer_type (bitsize, true);
+ t = fold_build1 (VIEW_CONVERT_EXPR, itype, t);
+ return fold_convert (pointer_sized_int_node, t);
+ }
+ default:
+ gcc_unreachable ();
+ }
+ else
+{
+ if (!TREE_ADDRESSABLE (t))
+ {
+ /* The reason for this is that we don't want to pessimize
+code by making vars unnecessarily addressable. */
+ tree var = create_tmp_var (type, NULL);
+ tree tem = build2 (MODIFY_EXPR, void_type_node, var, t);
+ if (in_expand_p)
+ {
+ SET_DECL_RTL (var,
+ assign_stack_temp_for_type (TYPE_MODE (type),
+ GET_MODE_SIZE (TYPE_MODE (type)), type));
Formatting looks wrong, I'd do:
rtx mem
= assign_stack_temp_for_type (TYPE_MODE (type),
GET_MODE_SIZE (TYPE_MODE (type)),
type);
SET_DECL_RTL (var, mem);
Ok.
+ return build_fold_addr_expr (var);
This is something I don't understand. What will assign the value to var
aka mem then?
Bah, fixed.
+ }
+ t = build_fold_addr_expr (var);
+ return build2 (COMPOUND_EXPR, TREE_TYPE (t), tem, t);
I would expect you want to use this too even if in_expand_p...
Unfortunately, this won't be sufficient -- in expand_expr_real_1 we
have
case COMPOUND_EXPR:
/* Lowered by gimplify.c. */
gcc_unreachable ();
and with the code above we hit that.
--- gcc/testsuite/c-c++-common/ubsan/pr59333.c.mp 2013-12-05
11:30:36.984759390 +0100
+++ gcc/testsuite/c-c++-common/ubsan/pr59333.c 2013-12-05
11:31:36.599979040 +0100
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options -fsanitize=undefined } */
+
+long long int
+foo (long long int i, long long int j)
+{
+ return i * j;
+}
Please add a runtime testcase that verifies the values
instead. As it is long long int, you are guaranteed
it is at least 64-bit, so just make the function __attribute__((noinline,
noclone)), pass some well chosen small constants so that it overflows
and dg-output whether the library prints the correct values.
Will do.
Marek
Re: [PATCH/AARCH64 4/6] Implement the trap pattern
Hi On 3 December 2013 21:24, Andrew Pinski pins...@gmail.com wrote: +(define_insn trap + [(trap_if (const_int 1) (const_int 8))] + + brk #1000) Please add a type attribute to the pattern. The type attributes are now shared between arm and aarch64 backends.You should use the type value introduced by this patch: http://gcc.gnu.org/ml/gcc-patches/2013-12/msg00374.html OK with this change. Thanks /Marcus
Re: [Patch,avr]: Fix wrong warning PR59396
Am 12/05/2013 04:09 PM, schrieb Richard Biener: On Thu, Dec 5, 2013 at 3:53 PM, Georg-Johann Lay wrote: This is a fix of a wrong warning for a bas ISR name. The assumption was that if DECL_ASSEMBLER_NAME is set, it would always starts with a *. This is not the case for LTO compiler where the assembler name is the plain name of the function (except an assembler name is set). That sounds odd to me. Does the bug reproduce with -fwhole-program? Or if the interrupt handler is static? Richard. Yes, with -fwhole-program the issue persists (except -flto is removed). Same with a static function, both with and without externally_visible. Because externally_visible conflicts static, I also tried without externally_visible, but with no avail. Johann Thus, do a more restrictive test if the first character of the function name has to be skipped. Ok to commit? Johann PR target/59396 * config/avr/avr.c (avr_set_current_function): If the first char of the function name is skipped, make sure it is actually '*'.
Re: [PATCH/AARCH64 3/6] Fix up multi-lib options
On 3 December 2013 21:24, Andrew Pinski pins...@gmail.com wrote: * config/aarch64/t-aarch64 (MULTILIB_OPTIONS): Fix definition so that options are conflicting ones. Looks fine to me, commit it. /Marcus
Re: [PATCH] Fix up passing long long in ubsan with -m32 (PR sanitizer/59333)
On Thu, Dec 05, 2013 at 04:31:20PM +0100, Marek Polacek wrote: + } + t = build_fold_addr_expr (var); + return build2 (COMPOUND_EXPR, TREE_TYPE (t), tem, t); I would expect you want to use this too even if in_expand_p... Unfortunately, this won't be sufficient -- in expand_expr_real_1 we have case COMPOUND_EXPR: /* Lowered by gimplify.c. */ gcc_unreachable (); and with the code above we hit that. Ah, ok, then perhaps you need to do expand_assignment (var, t, false); there and then just build_fold_addr_expr. The ugly thing is that those assignments will be expanded there right away, so you want to move the fn = ubsan_build_overflow_builtin (code, gimple_location (stmt), TREE_TYPE (arg0), arg0, arg1); lines down right above expand_normal (fn); Also, you want push_temp_slots (); and pop_temp_slots (); around it. So push_temp_slots (); fn = ubsan_build_overflow_builtin (code, gimple_location (stmt), TREE_TYPE (arg0), arg0, arg1); expand_normal (fn); pop_temp_slots (); That way if you have say multiple assign_stack_temp_for_type slots in one spot for one diagnostics, those slots can be reused again in another call. Jakub
Mention __auto_type in 4.9 release notes
I've applied this patch to include the new C extension __auto_type in the 4.9 release notes. Index: changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.9/changes.html,v retrieving revision 1.44 diff -u -r1.44 changes.html --- changes.html2 Dec 2013 21:44:13 - 1.44 +++ changes.html5 Dec 2013 16:06:49 - @@ -162,6 +162,9 @@ issues (mainly but not entirely relating to optional C99 features from Annexes F and G) and the optional Annexes K (Bounds-checking interfaces) and L (Analyzability)./li + + liA new C extension code__auto_type/code provides a subset of + the functionality of C++11 codeauto/code in GNU C./li /ul h3 id=cxxC++/h3 -- Joseph S. Myers jos...@codesourcery.com
Re: [Patch, RTL] Eliminate redundant vec_select moves.
Jeff Law wrote:
On 12/04/13 09:06, Tejas Belagod wrote:
Richard Sandiford wrote:
Tejas Belagod tbela...@arm.com writes:
Richard Sandiford wrote:
Tejas Belagod tbela...@arm.com writes:
The problem is that one reg rtx can span several hard registers.
E.g. (reg:V4SI 32) might represent one 64-bit register (no. 32),
but it might instead represent two 32-bit registers (nos. 32 and 33).
Obviously the latter's not very likely for vectors this small,
but more likely for larger ones (including on NEON IIRC).
So if we had 2 32-bit registers being treated as a V4HI, it would be:
--3233--
msb lsb
msb lsb
--32--
for big endian and:
--3332--
msb lsb
msb lsb
--32--
for little endian.
Ah, ok, that makes things clearer. Thanks for that.
I can't find any helper function that figures out if we're writing
partial or
full result regs. Would something like
REGNO (src) == REGNO (dst)
HARD_REGNO_NREGS (src) == HARD_REGNO_NREGS (dst) == 1
be a sane check for partial result regs?
Yeah, that should work. I think a more general alternative would be:
simplify_subreg_regno (REGNO (src), GET_MODE (src),
offset, GET_MODE (dst)) == (int) REGNO (dst)
where:
offset = GET_MODE_UNIT_SIZE (GET_MODE (src)) * INTVAL (XVECEXP
(sel, 0))
That offset is the byte offset of the first selected element from the
start of a vector in memory, which is also the way that SUBREG_BYTEs
are counted. For little-endian it gives the offset of the lsb of the
slice, while for big-endian it gives the offset of the msb (which is
also how SUBREG_BYTEs work).
The simplify_subreg_regno should cope with both single-register vectors
and multi-register vectors.
Sorry for the delayed response to this.
Thanks for the tip. Here's an improved patch that implements the
simplify_sureg_regno () method of eliminating redundant moves.
Regarding the test case, I failed to get the ppc back-end to generate
RTL pattern that this patch checks for. I can easily write a test
case for aarch64(big and little endian) on these lines
typedef float float32x4_t __attribute__ ((__vector_size__ (16)));
float foo_be (float32x4_t x)
{
return x[3];
}
float foo_le (float32x4_t x)
{
return x[0];
}
where I know that the vector indexing will generate a vec_select on
the same src and dst regs that could be optimized away and hence test
it. But I'm struggling to get a test case that the ppc altivec
back-end will generate such a vec_select for. I see that altivec does
not define vec_extract, so a simple indexing like this seems to happen
via memory. Also, I don't know enough about the ppc PCS or
architecture to write a test that will check for this optimization
opportunity on same src and dst hard-registers. Any hints?
Me neither, sorry.
FWIW, the MIPS tests:
typedef float float32x2_t __attribute__ ((__vector_size__ (8)));
void bar (float);
void foo_be (float32x2_t x) { bar (x[1]); }
void foo_le (float32x2_t x) { bar (x[0]); }
also exercise it, but I don't think they add anything over the aarch64
versions. I can add them to the testsuite anyway if it helps though.
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 0cd0c7e..ca25ce5 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -1180,6 +1180,22 @@ set_noop_p (const_rtx set)
dst = SUBREG_REG (dst);
}
+ /* It is a NOOP if destination overlaps with selected src vector
+ elements. */
+ if (GET_CODE (src) == VEC_SELECT
+ REG_P (XEXP (src, 0)) REG_P (dst)
+ HARD_REGISTER_P (XEXP (src, 0))
+ HARD_REGISTER_P (dst))
+{
+ rtx par = XEXP (src, 1);
+ rtx src0 = XEXP (src, 0);
+ HOST_WIDE_INT offset =
+GET_MODE_UNIT_SIZE (GET_MODE (src0)) * INTVAL (XVECEXP (par, 0,
0));
+
+ return simplify_subreg_regno (REGNO (src0), GET_MODE (src0),
+offset, GET_MODE (dst)) == (int)REGNO (dst);
+}
+
Since this also (correctly) triggers for vector results, we need to keep
the check for consecutive indices that you had originally. (It's always
the first index that should be used for the simplify_subreg_regno
though.)
Looks good to me otherwise, thanks.
Thanks Richard. Here is a revised patch. Sorry about the delay - I was
investigating to make sure an LRA ICE I was seeing on aarch64 was
unrelated to this patch. I've added a test case that I expect to pass
for aarch64. I've also added the tests that you suggested for MIPS, but
haven't checked for the target because I'm not sure what optimizations
happen on MIPS.
OK for trunk?
Thanks,
Tejas.
2013-12-04 Tejas Belagod tejas.bela...@arm.com
gcc/
* rtlanal.c (set_noop_p): Return nonzero in case of redundant
vec_select
for overlapping register lanes.
testsuite/
* config/gcc.dg/vect/vect-nop-move.c: New.
Per HJ's request please test vect-nop-move on x86/x86_64 and if the
Re: [Patch, AArch64] Relax CANNOT_CHANGE_MODE_CLASS.
Tejas Belagod wrote: Hi, Currently, CANNOT_CHANGE_MODE_CLASS is too restrictive wrt the mode-changes it allows on FPREGs - it allows none at the moment. In fact, there are many mode changes that are safe and can be allowed. For example, in a pattern like: (subreg:SF (reg:V4SF v0) 0) it is legal to reduce this to (reg:SF v0) The attached patch helps parts of rtlanal.c make such decisions(eg. simplify_subreg_regno). Tested on aarch64-none-elf and aarch64_be-none-elf. OK for trunk? Thanks, Tejas Belagod ARM. Changelog: 2013-11-28 Tejas Belagod tejas.bela...@arm.com gcc/ * config/aarch64/aarch64-protos.h (aarch64_cannot_change_mode_class): Declare. * config/aarch64/aarch64.c (aarch64_cannot_change_mode_class): New. * config/aarch64/aarch64.h (CANNOT_CHANGE_MODE_CLASS): Change to call backend function aarch64_cannot_change_mode_class. Hi, I'm testing a patch that is more general than the change presented here for CANNOT_CHANGE_MODE_CLASS. This patch is now defunct. Thanks, Tejas.
Re: [Patch, RTL] Eliminate redundant vec_select moves.
On 12/05/13 09:12, Tejas Belagod wrote: Now that Kirill's looking at why this doesn't work for x86, could I check this in without enabling vect-nop-move.c for targets (i?86-*-* x86_64-*-*)? If not, I'm happy to wait. Yea, that's fine with me. jeff
Re: [PATCH, ARM] Implement __builtin_trap
On 4 Dec 2013, at 16:08, Ian Bolton ian.bol...@arm.com wrote: Hi, Currently, on ARM, you have to either call abort() or raise(SIGTRAP) to achieve a handy crash. This patch allows you to instead call __builtin_trap() which is much more efficient at falling over because it becomes just a single instruction that will trap for you. Two testcases have been added (for ARM and Thumb) and both pass. Note: This is a modified version of a patch originally submitted by Mark Mitchell back in 2010, which came in response to PR target/59091. http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00639.html http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59091 The main update, other than cosmetic differences, is that we've chosen the same ARM encoding as LLVM for practical purposes. (The Thumb encoding in Mark's patch already matched LLVM.) OK for trunk? Cheers, Ian 2013-12-04 Ian Bolton ian.bol...@arm.com Mark Mitchell m...@codesourcery.com gcc/ * config/arm/arm.md (trap): New pattern. * config/arm/types.md: Added a type for trap. testsuite/ * gcc.target/arm/builtin-trap.c: New test. * gcc.target/arm/thumb-builtin-trap.c: Likewise. aarch32-builtin-trap-v2.txt This needs to set the conds attribute to unconditional. Otherwise the ARM backend might try to turn this into a conditional instruction. R.
FW: [PING]: [GOMP4] [PATCH] SIMD-Enabled Functions (formerly Elemental functions) for C
PING! -Balaji V. Iyer. -Original Message- From: Iyer, Balaji V Sent: Saturday, November 30, 2013 11:38 PM To: 'al...@redhat.com' Cc: 'Jakub Jelinek'; 'gcc-patches@gcc.gnu.org' Subject: RE: [PING]: [GOMP4] [PATCH] SIMD-Enabled Functions (formerly Elemental functions) for C Hello Aldy, Some of the middle end changes I made in the previous patch was not flying for the C++. Here is a fixed patch where the middle-end changes will work for both C and C++. With this email, I am attaching the patch for C along with the middle end changes. Is this Ok for the branch? Here are the ChangeLog entries: gcc/ChangeLog 2013-11-30 Balaji V. Iyer balaji.v.i...@intel.com * omp-low.c (expand_simd_clones): Added a new parameter called type. (ipa_omp_simd_clone): Added a call to expand_simd_clones when Cilk Plus is enabled. gcc/c-family/ChangeLog 2013-11-30 Balaji V. Iyer balaji.v.i...@intel.com * c-common.c (c_common_attribute_table): Added cilk plus elemental attribute. gcc/c/ChangeLog 2013-11-30 Balaji V. Iyer balaji.v.i...@intel.com * c-parser.c (struct c_parser::elem_fn_tokens): Added new field. (c_parser_declaration_or_fndef): Added a check if elem_fn_tokens field in parser is not empty. If not-empty, call the function c_parser_finish_omp_declare_simd. (c_parser_elem_fn_vectorlength): New function. (c_parser_elem_fn_expr_list): Likewise. (c_finish_elem_fn_tokens): Likewise. (c_parser_attributes): Added a elem_fn_tokens parameter. Added a check for vector attribute and if so call c_parser_elem_fn_expr_list. Also, called c_finish_elem_fn_tokens when Cilk Plus is enabled. (c_finish_omp_declare_simd): Added a check if elem_fn_tokens in parser field is non-empty. If so, parse them as you would parse the omp declare simd pragma. gcc/testsuite/ChangeLog 2013-11-30 Balaji V. Iyer balaji.v.i...@intel.com * c-c++-common/cilk-plus/EF/ef_test.c: New test. * c-c++-common/cilk-plus/EF/ef_test2.c: Likewise. * c-c++-common/cilk-plus/EF/vlength_errors.c: Likewise. * c-c++-common/cilk-plus/EF/ef_error.c: Likewise. * c-c++-common/cilk-plus/EF/ef_error2.c: Likewise. * gcc.dg/cilk-plus/cilk-plus.exp: Added calls for the above tests. Thanks, Balaji V. Iyer. -Original Message- From: Iyer, Balaji V Sent: Wednesday, November 27, 2013 1:15 PM To: al...@redhat.com Cc: Jakub Jelinek; gcc-patches@gcc.gnu.org Subject: RE: [PING]: [GOMP4] [PATCH] SIMD-Enabled Functions (formerly Elemental functions) for C HI Aldy and Jakub, Attached, please find a fixed patch. I have fixed all the changes you have mentioned below. Is this OK to install? Here are the ChangeLog entries: gcc/ChangeLog 2013-11-27 Balaji V. Iyer balaji.v.i...@intel.com * config/i386/i386.c (ix86_simd_clone_compute_vecsize_and_simdlen): Removed a carriage return from the warning string. * omp-low.c (simd_clone_clauses_extract): Added a check for cilk plus SIMD-enabled function attributes. gcc/c/ChangeLog 2013-11-27 Balaji V. Iyer balaji.v.i...@intel.com * c-parser.c (struct c_parser::elem_fn_tokens): Added new field. (c_parser_declaration_or_fndef): Added a check if elem_fn_tokens field in parser is not empty. If not-empty, call the function c_parser_finish_omp_declare_simd. (c_parser_elem_fn_vectorlength): New function. (c_parser_elem_fn_expr_list): Likewise. (c_finish_elem_fn_tokens): Likewise. (c_parser_attributes): Added a elem_fn_tokens parameter. Added a check for vector attribute and if so call c_parser_elem_fn_expr_list. Also, called c_finish_elem_fn_tokens when Cilk Plus is enabled. (c_finish_omp_declare_simd): Added a check if elem_fn_tokens in parser field is non-empty. If so, parse them as you would parse the omp declare simd pragma. gcc/testsuite/ChangeLog 2013-11-27 Balaji V. Iyer balaji.v.i...@intel.com * c-c++-common/cilk-plus/EF/ef_test.c: New test. * c-c++-common/cilk-plus/EF/ef_test2.c: Likewise. * c-c++-common/cilk-plus/EF/vlength_errors.c: Likewise. * c-c++-common/cilk-plus/EF/ef_error.c: Likewise. * c-c++-common/cilk-plus/EF/ef_error2.c: Likewise. * gcc.dg/cilk-plus/cilk-plus.exp: Added calls for the above tests. Thanks, Balaji V. Iyer. -Original Message- From: Aldy Hernandez [mailto:al...@redhat.com] Sent: Wednesday, November 27, 2013 10:52 AM To: Iyer, Balaji V Cc: Jakub Jelinek; gcc-patches@gcc.gnu.org Subject: Re: [PING]: [GOMP4] [PATCH] SIMD-Enabled Functions (formerly Elemental functions) for C Iyer, Balaji V
FW: [GOMP4][PATCH] SIMD-enabled functions (formerly Elemental functions) for C++
PING!
-Balaji V. Iyer.
-Original Message-
From: Iyer, Balaji V
Sent: Saturday, November 30, 2013 11:53 PM
To: 'Jakub Jelinek'
Cc: Aldy Hernandez (al...@redhat.com); 'Jeff Law'; 'gcc-
patc...@gcc.gnu.org'
Subject: RE: [GOMP4][PATCH] SIMD-enabled functions (formerly Elemental
functions) for C++
Hello Everyone,
The changes mentioned in http://gcc.gnu.org/ml/gcc-patches/2013-
11/msg03506.html is also applicable to my C++ patch. With this email, I am
attaching a fixed patch.
Here are the ChangeLog entries:
gcc/cp/ChangeLog
2013-11-30 Balaji V. Iyer balaji.v.i...@intel.com
* decl2.c (is_late_template_attribute): Added a check for SIMD-enabled
functions attribute. If found, return true.
* parser.c (cp_parser_direct_declarator): When Cilk Plus is enabled
see if there is an attribute after function decl. If so, then
parse them now.
(cp_parser_late_return_type_opt): Handle parsing of Cilk Plus SIMD
enabled function late parsing.
(cp_parser_gnu_attribute_list): Parse all the tokens for the vector
attribute for a SIMD-enabled function.
(cp_parser_omp_all_clauses): Skip parsing to the end of pragma when
the function is used by SIMD-enabled function (indicated by NULL
pragma token).
(cp_parser_elem_fn_vectorlength): New function.
(cp_parser_elem_fn_expr_list): Likewise.
(cp_parser_late_parsing_elem_fn_info): Likewise.
* parser.h (cp_parser::elem_fn_info): New field.
* decl.c (grokfndecl): Added a check if Cilk Plus is enabled and
if so, adjust the Cilk Plus SIMD-enabled function attributes.
gcc/testsuite/ChangeLog
2013-11-30 Balaji V. Iyer balaji.v.i...@intel.com
* g++.dg/cilk-plus/cilk-plus.exp: Called the C/C++ common tests for
SIMD enabled function.
* g++.dg/cilk-plus/ef_test.C: New test.
Is this OK for branch?
Thanks,
Balaji V. Iyer.
-Original Message-
From: Iyer, Balaji V
Sent: Wednesday, November 20, 2013 6:19 PM
To: Jakub Jelinek
Cc: Aldy Hernandez (al...@redhat.com); Jeff Law;
gcc-patches@gcc.gnu.org
Subject: [GOMP4][PATCH] SIMD-enabled functions (formerly Elemental
functions) for C++
Hello Everyone,
Attached, please find a patch that will implement SIMD-enabled
functions for C++ targeting the gomp-4_0-branch. Here are the
Changelog entries. Is this OK to install?
gcc/cp/ChangeLog
2013-11-20 Balaji V. Iyer balaji.v.i...@intel.com
* parser.c (cp_parser_direct_declarator): When Cilk Plus is enabled
see if there is an attribute after function decl. If so, then
parse them now.
(cp_parser_late_return_type_opt): Handle parsing of Cilk Plus SIMD
enabled function late parsing.
(cp_parser_gnu_attribute_list): Parse all the tokens for the vector
attribute for a SIMD-enabled function.
(cp_parser_omp_all_clauses): Skip parsing to the end of pragma when
the function is used by SIMD-enabled function (indicated by NULL
pragma token).
(cp_parser_elem_fn_vectorlength): New function.
(cp_parser_elem_fn_expr_list): Likewise.
(cp_parser_late_parsing_elem_fn_info): Likewise.
* parser.h (cp_parser::elem_fn_info): New field.
gcc/testsuite/ChangeLog
2013-11-20 Balaji V. Iyer balaji.v.i...@intel.com
* g++.dg/cilk-plus/cilk-plus.exp: Called the C/C++ common tests for
SIMD enabled function.
* g++.dg/cilk-plus/ef_test.C: New test.
Thanking You,
Yours Sincerely,
Balaji V. Iyer.
Index: gcc/cp/decl.c
===
--- gcc/cp/decl.c (revision 205562)
+++ gcc/cp/decl.c (working copy)
@@ -7669,6 +7669,34 @@
}
}
+ if (flag_enable_cilkplus)
+{
+ /* Adjust cilk plus elemental attribute attributes. */
+ tree ods = lookup_attribute (cilk plus elemental, *attrlist);
+ if (ods)
+ {
+ tree attr;
+ for (attr = ods; attr;
+ attr = lookup_attribute (cilk plus elemental,
+ TREE_CHAIN (attr)))
+ {
+ if (TREE_CODE (type) == METHOD_TYPE)
+ walk_tree (TREE_VALUE (attr), declare_simd_adjust_this,
+ DECL_ARGUMENTS (decl), NULL);
+ if (TREE_VALUE (attr) != NULL_TREE)
+ {
+ tree cl = TREE_VALUE (TREE_VALUE (attr));
+ cl = c_omp_declare_simd_clauses_to_numbers
+ (DECL_ARGUMENTS (decl), cl);
+ if (cl)
+ TREE_VALUE (TREE_VALUE (attr)) = cl;
+ else
+ TREE_VALUE (attr) = NULL_TREE;
+ }
+ }
+ }
+}
+
/* Caller will do the rest of this. */
if
Re: [buildrobot] Re: [PATCH] Split -fisolate-erroneous-paths into two options
On Thu, Dec 5, 2013 at 12:18 AM, Jan-Benedict Glaw jbg...@lug-owl.de wrote: On Wed, 2013-12-04 20:19:29 -0700, Jeff Law l...@redhat.com wrote: This patch splits up the erroneous path optimization into two pieces. One which detects NULL dereferences and isolates those paths and a second which detects passing/returning a NULL pointer in cases where an attribute says a non-NULL value is required. [...] This seems to break Go, see eg. http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=50428 : g++ -c -DDEFAULT_TARGET_VERSION=\4.9.0\ -DDEFAULT_TARGET_MACHINE=\i686-pc-linux-gnu\ -DIN_GCC_FRONTEND -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -I. -Igo -I../../../gcc/gcc -I../../../gcc/gcc/go -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include -I/opt/cfarm/mpc/include -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/bid -I../libdecnumber -I../../../gcc/gcc/../libbacktrace-o go/go-lang.o -MT go/go-lang.o -MMD -MP -MF go/.deps/go-lang.TPo ../../../gcc/gcc/go/go-lang.c ../../../gcc/gcc/go/go-lang.c: In function ‘bool go_langhook_post_options(const char**)’: ../../../gcc/gcc/go/go-lang.c:276:27: error: ‘struct gcc_options’ has no member named ‘x_flag_isolate_erroneous_paths’ if (!global_options_set.x_flag_isolate_erroneous_paths) ^ ../../../gcc/gcc/go/go-lang.c:277:20: error: ‘struct gcc_options’ has no member named ‘x_flag_isolate_erroneous_paths’ global_options.x_flag_isolate_erroneous_paths = 0; ^ make[2]: *** [go/go-lang.o] Error 1 I've committed this patch to mainline to fix this problem. I could have removed this code earlier but hadn't gotten around to it. Ian 2013-12-05 Ian Lance Taylor i...@google.com Revert this change; no longer required. 2013-11-06 Ian Lance Taylor i...@google.com * go-lang.c (go_langhook_post_options): If -fisolate-erroneous-paths was turned on by an optimization option, turn it off. Index: gcc/go/go-lang.c === --- gcc/go/go-lang.c (revision 205710) +++ gcc/go/go-lang.c (working copy) @@ -270,12 +270,6 @@ go_langhook_post_options (const char **p if (flag_excess_precision_cmdline == EXCESS_PRECISION_DEFAULT) flag_excess_precision_cmdline = EXCESS_PRECISION_STANDARD; - /* The isolate_erroneous_paths optimization can change a nil - dereference from a panic to a trap, so we have to disable it for - Go, even though it is normally enabled by -O2. */ - if (!global_options_set.x_flag_isolate_erroneous_paths) -global_options.x_flag_isolate_erroneous_paths = 0; - /* Returning false means that the backend should be used. */ return false; }
[SPARC] Fix PR target/59316
This is the failure of gcc.dg/atomic/c11-atomic-exec-5.c on the SPARC, because
of the missing TARGET_ATOMIC_ASSIGN_EXPAND_FENV hook. The implementation is
heavily inspired from that of glibc; it seems to work fine on Solaris too,
except that the FE_* macros are different from the Linux ones, so an internal
macro is added to parameterize the hook. According to a quick browsing of the
sources, all the BSDs use the same values as Linux for the macros.
The patch also revamps the support code for builtins to make it work with LTO.
I'll very likely backport this part to the 4.8 branch at some point.
Tested on SPARC/Solaris. Joseph, do you have any comments?
2013-12-05 Eric Botcazou ebotca...@adacore.com
PR target/59316
* config/sparc/sparc.h (SPARC_LOW_FE_EXCEPT_VALUES): Define.
* config/sparc/sol2.h (SPARC_LOW_FE_EXCEPT_VALUES): Redefine.
* config/sparc/sparc.c (TARGET_INIT_BUILTINS): Move around.
(TARGET_BUILTIN_DECL): Define.
(TARGET_ATOMIC_ASSIGN_EXPAND_FENV): Likewise.
(sparc32_initialize_trampoline): Adjust call to gen_flush.
(enum sparc_builtins): New enumeral type.
(sparc_builtins): New static array.
(sparc_builtins_icode): Likewise.
(def_builtin): Accept a separate icode and save the result.
(def_builtin_const): Likewise.
(sparc_fpu_init_builtins): New function.
(sparc_vis_init_builtins): Pass the builtin code.
(sparc_init_builtins): Call it if TARGET_FPU.
(sparc_builtin_decl): New function.
(sparc_expand_builtin): Deal with SPARC_BUILTIN_{LD,ST}FSR.
(sparc_handle_vis_mul8x16): Use the builtin code.
(sparc_fold_builtin): Likewise. Deal with SPARC_BUILTIN_{LD,ST}FSR
and SPARC_BUILTIN_PDISTN.
(compound_expr): New helper function.
(sparc_atomic_assign_expand_fenv): New function.
* config/sparc/sparc.md (unspecv): Reorder values, add UNSPECV_LDFSR
and UNSPECV_STFSR.
(flush, flushdi): Merge into single pattern.
(ldfsr): New instruction.
(stfsr): Likewise.
2013-12-05 Eric Botcazou ebotca...@adacore.com
* gcc.target/sparc/pdistn.c: New test.
* gcc.target/sparc/pdistn-2.c: Likewise.
--
Eric BotcazouIndex: config/sparc/sparc.md
===
--- config/sparc/sparc.md (revision 205689)
+++ config/sparc/sparc.md (working copy)
@@ -96,13 +96,19 @@ (define_c_enum unspec [
(define_c_enum unspecv [
UNSPECV_BLOCKAGE
+ UNSPECV_PROBE_STACK_RANGE
+
UNSPECV_FLUSHW
- UNSPECV_FLUSH
UNSPECV_SAVEW
- UNSPECV_CAS
- UNSPECV_SWAP
+
+ UNSPECV_FLUSH
+
UNSPECV_LDSTUB
- UNSPECV_PROBE_STACK_RANGE
+ UNSPECV_SWAP
+ UNSPECV_CAS
+
+ UNSPECV_LDFSR
+ UNSPECV_STFSR
])
(define_constants
@@ -6783,22 +6789,26 @@ (define_insn flush_register_windows
;; Special pattern for the FLUSH instruction.
-; We do SImode and DImode versions of this to quiet down genrecog's complaints
-; of the define_insn otherwise missing a mode. We make flush, aka
-; gen_flush, the default one since sparc_initialize_trampoline uses
-; it on SImode mem values.
-
-(define_insn flush
- [(unspec_volatile [(match_operand:SI 0 memory_operand m)] UNSPECV_FLUSH)]
+(define_insn flushP:mode
+ [(unspec_volatile [(match_operand:P 0 memory_operand m)] UNSPECV_FLUSH)]
{ return TARGET_V9 ? flush\t%f0 : iflush\t%f0; }
[(set_attr type iflush)])
-(define_insn flushdi
- [(unspec_volatile [(match_operand:DI 0 memory_operand m)] UNSPECV_FLUSH)]
-
- { return TARGET_V9 ? flush\t%f0 : iflush\t%f0; }
- [(set_attr type iflush)])
+;; Special insns to load and store the 32-bit FP Status Register.
+
+(define_insn ldfsr
+ [(unspec_volatile [(match_operand:SI 0 memory_operand m)] UNSPECV_LDFSR)]
+ TARGET_FPU
+ ld\t%0, %%fsr
+ [(set_attr type load)])
+
+(define_insn stfsr
+ [(set (match_operand:SI 0 memory_operand =m)
+(unspec_volatile:SI [(const_int 0)] UNSPECV_STFSR))]
+ TARGET_FPU
+ st\t%%fsr, %0
+ [(set_attr type store)])
;; Find first set instructions.
Index: config/sparc/sparc.c
===
--- config/sparc/sparc.c (revision 205689)
+++ config/sparc/sparc.c (working copy)
@@ -570,11 +570,11 @@ static void emit_hard_tfmode_operation (
static bool sparc_function_ok_for_sibcall (tree, tree);
static void sparc_init_libfuncs (void);
static void sparc_init_builtins (void);
+static void sparc_fpu_init_builtins (void);
static void sparc_vis_init_builtins (void);
+static tree sparc_builtin_decl (unsigned, bool);
static rtx sparc_expand_builtin (tree, rtx, rtx, enum machine_mode, int);
static tree sparc_fold_builtin (tree, int, tree *, bool);
-static int sparc_vis_mul8x16 (int, int);
-static void sparc_handle_vis_mul8x16 (tree *, int, tree, tree, tree);
static void sparc_output_mi_thunk (FILE *, tree, HOST_WIDE_INT,
HOST_WIDE_INT, tree);
static bool
Re: [buildrobot] Re: [PATCH] Split -fisolate-erroneous-paths into two options
On 12/05/13 09:41, Ian Lance Taylor wrote: On Thu, Dec 5, 2013 at 12:18 AM, Jan-Benedict Glaw jbg...@lug-owl.de wrote: On Wed, 2013-12-04 20:19:29 -0700, Jeff Law l...@redhat.com wrote: This patch splits up the erroneous path optimization into two pieces. One which detects NULL dereferences and isolates those paths and a second which detects passing/returning a NULL pointer in cases where an attribute says a non-NULL value is required. [...] This seems to break Go, see eg. http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=50428 : g++ -c -DDEFAULT_TARGET_VERSION=\4.9.0\ -DDEFAULT_TARGET_MACHINE=\i686-pc-linux-gnu\ -DIN_GCC_FRONTEND -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -I. -Igo -I../../../gcc/gcc -I../../../gcc/gcc/go -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include -I/opt/cfarm/mpc/include -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/bid -I../libdecnumber -I../../../gcc/gcc/../libbacktrace-o go/go-lang.o -MT go/go-lang.o -MMD -MP -MF go/.deps/go-lang.TPo ../../../gcc/gcc/go/go-lang.c ../../../gcc/gcc/go/go-lang.c: In function ‘bool go_langhook_post_options(const char**)’: ../../../gcc/gcc/go/go-lang.c:276:27: error: ‘struct gcc_options’ has no member named ‘x_flag_isolate_erroneous_paths’ if (!global_options_set.x_flag_isolate_erroneous_paths) ^ ../../../gcc/gcc/go/go-lang.c:277:20: error: ‘struct gcc_options’ has no member named ‘x_flag_isolate_erroneous_paths’ global_options.x_flag_isolate_erroneous_paths = 0; ^ make[2]: *** [go/go-lang.o] Error 1 I've committed this patch to mainline to fix this problem. I could have removed this code earlier but hadn't gotten around to it. Sorry, I should have remembered that GO was going to need updating. Q. Now that we're in stage3, presumably GO imports onto the trunk are stopping? If so would it make sense to enable GO by default? jeff
Re: RFC ThreadSanitizer tests
On Dec 4, 2013, at 10:12 PM, max m.ostape...@partner.samsung.com wrote: Here is a patch with initial ThreadSanitizer testsuite. It basically adds several tests from upstream LLVM testsuite. It works fine on x86_64 with patch from http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59188 applied. Ok to commit Ok. should we wait for fix for 59188? Not a strong opinion on this, but, I'd say, it seems like it should go in regardless. Don't know we are on shifting grounds, if we are not able/willing to look at the ground.
Re: [PATCH] Fix up passing long long in ubsan with -m32 (PR sanitizer/59333)
On Thu, Dec 05, 2013 at 04:44:29PM +0100, Jakub Jelinek wrote:
On Thu, Dec 05, 2013 at 04:31:20PM +0100, Marek Polacek wrote:
+ }
+ t = build_fold_addr_expr (var);
+ return build2 (COMPOUND_EXPR, TREE_TYPE (t), tem, t);
I would expect you want to use this too even if in_expand_p...
Unfortunately, this won't be sufficient -- in expand_expr_real_1 we
have
case COMPOUND_EXPR:
/* Lowered by gimplify.c. */
gcc_unreachable ();
and with the code above we hit that.
Ah, ok, then perhaps you need to do expand_assignment (var, t, false);
there and then just build_fold_addr_expr. The ugly thing is that those
assignments will be expanded there right away, so you want to move
the
fn = ubsan_build_overflow_builtin (code, gimple_location (stmt),
TREE_TYPE (arg0), arg0, arg1);
lines down right above expand_normal (fn);
Also, you want push_temp_slots (); and pop_temp_slots (); around it.
So
push_temp_slots ();
fn = ubsan_build_overflow_builtin (code, gimple_location (stmt),
TREE_TYPE (arg0), arg0, arg1);
expand_normal (fn);
pop_temp_slots ();
That way if you have say multiple assign_stack_temp_for_type slots
in one spot for one diagnostics, those slots can be reused again
in another call.
Thanks, that seems to work. The version below fixes PR59397 reported today
as well -- we weren't handling ENUMERAL_TYPE in the ubsan_encode_value.
Bootstrapped, ran ubsan testsuite on x86_64-unknown-linux-gnu.
Ok for trunk?
2013-12-05 Marek Polacek pola...@redhat.com
PR sanitizer/59333
PR sanitizer/59397
* ubsan.c: Include rtl.h and expr.h.
(ubsan_encode_value): Add new parameter. If expanding, assign
a stack slot for DECL_RTL of the temporary and call expand_assignment.
Handle BOOLEAN_TYPE and ENUMERAL_TYPE.
(ubsan_build_overflow_builtin): Adjust ubsan_encode_value call.
* ubsan.h (ubsan_encode_value): Adjust declaration.
* internal-fn.c (ubsan_expand_si_overflow_addsub_check): Move
ubsan_build_overflow_builtin above expand_normal call. Surround this
call
with push_temp_slots and pop_temp_slots.
(ubsan_expand_si_overflow_neg_check): Likewise.
(ubsan_expand_si_overflow_mul_check): Likewise.
testsuite/
* c-c++-common/ubsan/pr59333.c: New test.
* c-c++-common/ubsan/pr59397.c: New test.
--- gcc/ubsan.h.mp 2013-12-05 11:25:18.979469651 +0100
+++ gcc/ubsan.h 2013-12-05 12:53:30.970741286 +0100
@@ -41,7 +41,7 @@ extern tree ubsan_instrument_unreachable
extern tree ubsan_create_data (const char *, location_t,
const struct ubsan_mismatch_data *, ...);
extern tree ubsan_type_descriptor (tree, bool);
-extern tree ubsan_encode_value (tree);
+extern tree ubsan_encode_value (tree, bool = false);
extern bool is_ubsan_builtin_p (tree);
extern tree ubsan_build_overflow_builtin (tree_code, location_t, tree, tree,
tree);
--- gcc/ubsan.c.mp 2013-12-05 09:40:39.676771966 +0100
+++ gcc/ubsan.c 2013-12-05 17:47:35.409077317 +0100
@@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.
#include cgraph.h
#include tree-pass.h
#include tree-ssa-alias.h
+#include tree-pretty-print.h
#include internal-fn.h
#include gimple-expr.h
#include gimple.h
@@ -40,6 +41,8 @@ along with GCC; see the file COPYING3.
#include cfgloop.h
#include ubsan.h
#include c-family/c-common.h
+#include rtl.h
+#include expr.h
/* Map from a tree to a VAR_DECL tree. */
@@ -102,45 +105,53 @@ decl_for_type_insert (tree type, tree de
/* Helper routine, which encodes a value in the pointer_sized_int_node.
Arguments with precision = POINTER_SIZE are passed directly,
- the rest is passed by reference. T is a value we are to encode. */
+ the rest is passed by reference. T is a value we are to encode.
+ IN_EXPAND_P is true if this function is called during expansion. */
tree
-ubsan_encode_value (tree t)
+ubsan_encode_value (tree t, bool in_expand_p)
{
tree type = TREE_TYPE (t);
- switch (TREE_CODE (type))
-{
-case INTEGER_TYPE:
- if (TYPE_PRECISION (type) = POINTER_SIZE)
+ const unsigned int bitsize = GET_MODE_BITSIZE (TYPE_MODE (type));
+ if (bitsize = POINTER_SIZE)
+switch (TREE_CODE (type))
+ {
+ case BOOLEAN_TYPE:
+ case ENUMERAL_TYPE:
+ case INTEGER_TYPE:
return fold_build1 (NOP_EXPR, pointer_sized_int_node, t);
+ case REAL_TYPE:
+ {
+ tree itype = build_nonstandard_integer_type (bitsize, true);
+ t = fold_build1 (VIEW_CONVERT_EXPR, itype, t);
+ return fold_convert (pointer_sized_int_node, t);
+ }
+ default:
+ gcc_unreachable ();
+ }
+ else
+{
+ if (!DECL_P (t) || !TREE_ADDRESSABLE (t))
+ {
+ /* The reason for this is that we don't want to pessimize
+code by making
