[Committed] S/390: Fix PR55718: Handle GOTENT + constant for pic addresses
Hi,
I've committed the attached patch which fixes PR 55718 by adding
support for GOTENT + constant to legitimize_pic_address in the S/390
backend.
The patch also does some rearrangements of the whole function in order
to avoid some duplicate code.
Bye,
-Andreas-
2013-01-10 Andreas Krebbel andreas.kreb...@de.ibm.com
PR target/55718
* config/s390/s390.c (s390_symref_operand_p)
(s390_loadrelative_operand_p): Merge the two functions.
(s390_check_qrst_address, print_operand_address): Add parameters
to s390_loadrelative_operand_p invokation.
(s390_check_symref_alignment): Use s390_loadrelative_operand_p.
(s390_reload_larl_operand, s390_secondary_reload): Use
s390_loadrelative_operand_p instead of s390_symref_operand_p.
(legitimize_pic_address): Handle @GOTENT and @PLT + addend.
---
gcc/config/s390/s390.c | 474 ++-
gcc/testsuite/gcc.target/s390/pr55718.c | 29 +
2 files changed, 65 insertions(+), 10 deletions(-), 428 modifications(!)
Index: gcc/config/s390/s390.c
===
*** gcc/config/s390/s390.c.orig
--- gcc/config/s390/s390.c
*** s390_legitimate_address_without_index_p
*** 2148,2160
}
! /* Return true if ADDR is of kind symbol_ref or symbol_ref + const_int
!and return these parts in SYMREF and ADDEND. You can pass NULL in
!SYMREF and/or ADDEND if you are not interested in these values.
!Literal pool references are *not* considered symbol references. */
static bool
! s390_symref_operand_p (rtx addr, rtx *symref, HOST_WIDE_INT *addend)
{
HOST_WIDE_INT tmpaddend = 0;
--- 2148,2164
}
! /* Return TRUE if ADDR is an operand valid for a load/store relative
!instruction. Be aware that the alignment of the operand needs to
!be checked separately.
!Valid addresses are single references or a sum of a reference and a
!constant integer. Return these parts in SYMREF and ADDEND. You can
!pass NULL in REF and/or ADDEND if you are not interested in these
!values. Literal pool references are *not* considered symbol
!references. */
static bool
! s390_loadrelative_operand_p (rtx addr, rtx *symref, HOST_WIDE_INT *addend)
{
HOST_WIDE_INT tmpaddend = 0;
*** s390_symref_operand_p (rtx addr, rtx *sy
*** 2163,2205
if (GET_CODE (addr) == PLUS)
{
! if (GET_CODE (XEXP (addr, 0)) == SYMBOL_REF
! !CONSTANT_POOL_ADDRESS_P (XEXP (addr, 0))
! CONST_INT_P (XEXP (addr, 1)))
! {
! tmpaddend = INTVAL (XEXP (addr, 1));
! addr = XEXP (addr, 0);
! }
! else
return false;
- }
- else
- if (GET_CODE (addr) != SYMBOL_REF || CONSTANT_POOL_ADDRESS_P (addr))
- return false;
-
- if (symref)
- *symref = addr;
- if (addend)
- *addend = tmpaddend;
! return true;
! }
!
! /* Return TRUE if ADDR is an operand valid for a load/store relative
!instructions. Be aware that the alignment of the operand needs to
!be checked separately. */
! static bool
! s390_loadrelative_operand_p (rtx addr)
! {
! if (GET_CODE (addr) == CONST)
! addr = XEXP (addr, 0);
! /* Enable load relative for symbol@GOTENT. */
! if (GET_CODE (addr) == UNSPEC
!XINT (addr, 1) == UNSPEC_GOTENT)
! return true;
! return s390_symref_operand_p (addr, NULL, NULL);
}
/* Return true if the address in OP is valid for constraint letter C
--- 2167,2192
if (GET_CODE (addr) == PLUS)
{
! if (!CONST_INT_P (XEXP (addr, 1)))
return false;
! tmpaddend = INTVAL (XEXP (addr, 1));
! addr = XEXP (addr, 0);
! }
! if ((GET_CODE (addr) == SYMBOL_REF !CONSTANT_POOL_ADDRESS_P (addr))
! || (GET_CODE (addr) == UNSPEC
! (XINT (addr, 1) == UNSPEC_GOTENT
! || (TARGET_CPU_ZARCH XINT (addr, 1) == UNSPEC_PLT
! {
! if (symref)
! *symref = addr;
! if (addend)
! *addend = tmpaddend;
! return true;
! }
! return false;
}
/* Return true if the address in OP is valid for constraint letter C
*** s390_check_qrst_address (char c, rtx op,
*** 2215,2221
/* This check makes sure that no symbolic address (except literal
pool references) are accepted by the R or T constraints. */
! if (s390_loadrelative_operand_p (op))
return 0;
/* Ensure literal pool references are only accepted if LIT_POOL_OK. */
--- 2202,2208
/* This check makes sure that no symbolic address (except literal
pool references) are accepted by the R or T constraints. */
! if (s390_loadrelative_operand_p (op, NULL, NULL))
return 0;
/* Ensure literal pool references are only accepted if LIT_POOL_OK. */
*** s390_check_symref_alignment (rtx addr, H
*** 3022,3039
Re: [PATCH] Fix PR55882
On Wed, 9 Jan 2013, Eric Botcazou wrote:
This fixes PR55882 - set_mem_attributes_minus_bitpos misses to
account for the to-be applied bitpos when computing MEM_ALIGN.
It extracts alignment from 't' instead of t - bitpos.
Bootstrapped and tested on x86_64-unknown-linux-gnu, bootstrap
and regtest running on mips.
Does this look sensible?
I'm not sure, bitpos isn't taken into account in the other cases when the
alignment is computed. adjust_address_1 is supposed to properly adjust the
alignment by the time the offset is applied.
Yes, but alignment adjustment assumes the alignment is that of
the actual MEM, not that of the MEM_EXPR. Thus, MEM_ALIGN is the
alignment of MEM_EXPR - MEM_OFFSET.
It's true that the other paths setting align_computed have the
same issue. I can produce a followup (or preparatory patch)
that at least removes the code that is redundant. For example
/* If this is a decl, set the attributes of the MEM from it. */
if (DECL_P (t))
{
attrs.expr = t;
attrs.offset_known_p = true;
attrs.offset = 0;
apply_bitpos = bitpos;
new_size = DECL_SIZE_UNIT (t);
attrs.align = DECL_ALIGN (t);
align_computed = true;
}
the alignment computation can be handled by the get_object_alignment
path just fine, no need to duplicate it here, likewise for the
CONSTANT_CLASS_P case. The ARRAY_REF path is scary enough to
me (I'd rather drop it completely ... it's probably designed to
make more cases covered by nonoverlapping_component_refs_p?).
At least handling of DECL/COMPONENT_REF below the ARRAY_REFs
should be commonized - for example the COMPONENT_REF case
else if (TREE_CODE (t2) == COMPONENT_REF)
{
attrs.expr = t2;
attrs.offset_known_p = false;
if (host_integerp (off_tree, 1))
{
attrs.offset_known_p = true;
attrs.offset = tree_low_cst (off_tree, 1);
apply_bitpos = bitpos;
}
doesn't adjust 't' nor does it compute alignment. So it
clearly cannot be that MEM_ALIGN is supposed to be the
alignment of MEM_EXPR.
That is - the question boils down to _what_ should MEM_ALIGN
be the alignment of? In all the rest of the compiler it surely
is the alignment of XEXP (mem, 0) - to make it differ from that
during a brief period of expand_assignment sounds fragile at least.
I'll try to simplify code paths without changing behavior first.
Thanks,
Richard.
Re: [PATCH] Fix complex lowering with inline asm (PR tree-optimization/55921)
On Thu, 10 Jan 2013, Jakub Jelinek wrote:
Hi!
We weren't processing GIMPLE_ASMs that set complex SSA_NAMEs, which lead to
SSA_NAMEs with NULL SSA_NAME_DEF_STMT, either leading to crashes or silent
wrong code, depending on --enable-checking. Fixed thusly,
bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Thanks,
Richard.
2013-01-09 Jakub Jelinek ja...@redhat.com
PR tree-optimization/55921
* tree-complex.c (expand_complex_asm): New function.
(expand_complex_operations_1): Call it for GIMPLE_ASM.
* gcc.c-torture/compile/pr55921.c: New test.
--- gcc/tree-complex.c.jj 2013-01-04 13:44:35.0 +0100
+++ gcc/tree-complex.c2013-01-09 20:34:55.595674683 +0100
@@ -1391,6 +1391,36 @@ expand_complex_comparison (gimple_stmt_i
update_stmt (stmt);
}
+/* Expand inline asm that sets some complex SSA_NAMEs. */
+
+static void
+expand_complex_asm (gimple_stmt_iterator *gsi)
+{
+ gimple stmt = gsi_stmt (*gsi);
+ unsigned int i;
+
+ for (i = 0; i gimple_asm_noutputs (stmt); ++i)
+{
+ tree link = gimple_asm_output_op (stmt, i);
+ tree op = TREE_VALUE (link);
+ if (TREE_CODE (op) == SSA_NAME
+TREE_CODE (TREE_TYPE (op)) == COMPLEX_TYPE)
+ {
+ tree type = TREE_TYPE (op);
+ tree inner_type = TREE_TYPE (type);
+ tree r = build1 (REALPART_EXPR, inner_type, op);
+ tree i = build1 (IMAGPART_EXPR, inner_type, op);
+ gimple_seq list = set_component_ssa_name (op, false, r);
+
+ if (list)
+ gsi_insert_seq_after (gsi, list, GSI_CONTINUE_LINKING);
+
+ list = set_component_ssa_name (op, true, i);
+ if (list)
+ gsi_insert_seq_after (gsi, list, GSI_CONTINUE_LINKING);
+ }
+}
+}
/* Process one statement. If we identify a complex operation, expand it. */
@@ -1403,6 +1433,12 @@ expand_complex_operations_1 (gimple_stmt
complex_lattice_t al, bl;
enum tree_code code;
+ if (gimple_code (stmt) == GIMPLE_ASM)
+{
+ expand_complex_asm (gsi);
+ return;
+}
+
lhs = gimple_get_lhs (stmt);
if (!lhs gimple_code (stmt) != GIMPLE_COND)
return;
--- gcc/testsuite/gcc.c-torture/compile/pr55921.c.jj 2013-01-09
20:37:33.643755543 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr55921.c 2013-01-09
20:37:07.0 +0100
@@ -0,0 +1,21 @@
+/* PR tree-optimization/55921 */
+
+typedef union
+{
+ _Complex float cf;
+ long long ll;
+} ucf;
+
+void
+foo (ucf *in, ucf *out, _Complex float r)
+{
+ int i;
+ ucf ucf1;
+ _Complex float cf;
+
+ ucf1.ll = in[i].ll;
+ __asm ( : =r (cf) : 0 (ucf1.ll));
+ cf *= r;
+ __asm ( : =r (ucf1.ll) : 0 (cf));
+ out[i].ll = ucf1.ll;
+}
Jakub
--
Richard Biener rguent...@suse.de
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend
[wwwdocs] mention caret diagnostics and Wpedantic
I haven't heard any complaints about the caret diagnostics being enabled by default, so I am tempted to think it will stay like it is for GCC 4.8. This patch documents that and also the new -Wpedantic. OK? Cheers, Manuel. caret.diff Description: Binary data
Re: [PATCH] Fix PR55882
On Thu, 10 Jan 2013, Richard Biener wrote:
On Wed, 9 Jan 2013, Eric Botcazou wrote:
This fixes PR55882 - set_mem_attributes_minus_bitpos misses to
account for the to-be applied bitpos when computing MEM_ALIGN.
It extracts alignment from 't' instead of t - bitpos.
Bootstrapped and tested on x86_64-unknown-linux-gnu, bootstrap
and regtest running on mips.
Does this look sensible?
I'm not sure, bitpos isn't taken into account in the other cases when the
alignment is computed. adjust_address_1 is supposed to properly adjust the
alignment by the time the offset is applied.
Yes, but alignment adjustment assumes the alignment is that of
the actual MEM, not that of the MEM_EXPR. Thus, MEM_ALIGN is the
alignment of MEM_EXPR - MEM_OFFSET.
It's true that the other paths setting align_computed have the
same issue. I can produce a followup (or preparatory patch)
that at least removes the code that is redundant. For example
/* If this is a decl, set the attributes of the MEM from it. */
if (DECL_P (t))
{
attrs.expr = t;
attrs.offset_known_p = true;
attrs.offset = 0;
apply_bitpos = bitpos;
new_size = DECL_SIZE_UNIT (t);
attrs.align = DECL_ALIGN (t);
align_computed = true;
}
the alignment computation can be handled by the get_object_alignment
path just fine, no need to duplicate it here, likewise for the
CONSTANT_CLASS_P case. The ARRAY_REF path is scary enough to
me (I'd rather drop it completely ... it's probably designed to
make more cases covered by nonoverlapping_component_refs_p?).
At least handling of DECL/COMPONENT_REF below the ARRAY_REFs
should be commonized - for example the COMPONENT_REF case
else if (TREE_CODE (t2) == COMPONENT_REF)
{
attrs.expr = t2;
attrs.offset_known_p = false;
if (host_integerp (off_tree, 1))
{
attrs.offset_known_p = true;
attrs.offset = tree_low_cst (off_tree, 1);
apply_bitpos = bitpos;
}
doesn't adjust 't' nor does it compute alignment. So it
clearly cannot be that MEM_ALIGN is supposed to be the
alignment of MEM_EXPR.
That is - the question boils down to _what_ should MEM_ALIGN
be the alignment of? In all the rest of the compiler it surely
is the alignment of XEXP (mem, 0) - to make it differ from that
during a brief period of expand_assignment sounds fragile at least.
I'll try to simplify code paths without changing behavior first.
Going over this what's strange as well is that we adjust MEM_SIZE
with bitpos, too. At least when the MEM has non-BLKmode this
means that MEMs mode and MEM_SIZE is inconsistent? Or how do
a MEMs mode and MEM_SIZE relate?
I came up with the following patch that shouldn't change
behavior but which makes it easier to follow what happens
(it will also make bitpos handling with my alignment patch fix
consistent).
The patch moves attrs.align precompute into the !TYPE_P path
and for the DECL / CONSTANT align compute use get_object_alignment
which DTRT for it (it's still buggy because we later will use
MAX of this alignment and the correct alignment - but I didn't
want to change behavior here).
The patch removes explicit align computation on the DECL_P
and CONSTANT_CLASS_P paths because that's handled by
the get_object_alignment path in the same way. That also
allows the ARRAY_REF base paths to be simplified and commonized
(the ARRAY_REF and then DECL_P path will get better align
computation this way, get_object_alignment has more elaborate
off_tree handling).
As followup I'd remove the MEM_REF/TARGET_MEM_REF alignment
precompute completely - it's buggy (due to the later use of MAX).
That is, consider the below as explanatory intermediate step,
any real patch would get rid of that precompute immediately
(and then probably not re-use anything we got from default or
pre-existing MEM_ATTRs but simply overwrite attrs.align.
See 2nd patch below.
Richard.
2013-01-10 Richard Biener rguent...@suse.de
* emit-rtl.c (set_mem_attributes_minus_bitpos): Move MEM_REF
handling completely into the !TYPE_P handling part and use
get_object_alignment for its alignment computation. Always
rely on get_object_alignment for final alignment computation.
Index: gcc/emit-rtl.c
===
*** gcc/emit-rtl.c (revision 195079)
--- gcc/emit-rtl.c (working copy)
*** set_mem_attributes_minus_bitpos (rtx ref
*** 1641,1691
if (objectp || TREE_CODE (t) == INDIRECT_REF || TYPE_ALIGN_OK (type))
attrs.align = MAX (attrs.align, TYPE_ALIGN (type));
- else if (TREE_CODE (t) == MEM_REF)
- {
- tree op0 = TREE_OPERAND (t, 0);
- if (TREE_CODE (op0) == ADDR_EXPR
- (DECL_P (TREE_OPERAND (op0,
Re: v850 ld error
Hi Joel, Well Doh! The gcc-abi and 8byte multilib variants appeared to have been added around 2012-11-20 by Nick Clifton. I think he missed changing the ASM_SPEC in gcc/config/v850/rtems.h when he added it. v850-rtems-gcc wasn't passing in the expected cpu specification flags. Nick.. how does this patch to gcc/config/v850/rtems.h look? Perfect. Sorry about the snafu. I have checked the patch in along with this changelog entry. Cheers Nick gcc/ChangeLog 2013-01-10 Joel Sherrill joel.sherr...@oarcorp.com * config/v850/rtems.h (ASM_SPEC): Pass -m8byte-align and -mgcc-abi on to assembler.
Re: Minor testsuite improvement
On Thu, Jan 10, 2013 at 6:15 AM, Jeff Law l...@redhat.com wrote:
Gary Funck noted that vrp06.c has two tests with the same output. After
further investigation it was clear that expected output strings were too
lenient and were in fact masking a missed optimization.
This patch tightens the expected output from the vrp dump which has the side
effect of making each test's string unique. Obviously the masked failure is
xfailed.
OK for the trunk?
Hmm, but if the SSA versions are simply i_10 then i_.*0 will still match it
the same? I think you want the more elaborate 'i_\[0-9\]* . 0' here?
Please also open a bug for the added XFAIL.
Thanks,
Richard.
Thanks,
Jeff
* gcc/dg/tree-ssa/vrp06.c: Tighten expected output. Make each
pass/fail message unique.
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp06.c
b/gcc/testsuite/gcc.dg/tree-ssa/vrp06.c
index 1ef6c92..4edca36 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/vrp06.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp06.c
@@ -25,7 +25,7 @@ foo (int i, int j, int a)
return i + a + j;
}
-/* { dg-final { scan-tree-dump-times Folding predicate i_.*to 0 1 vrp1
} } */
-/* { dg-final { scan-tree-dump-times Folding predicate j_.*to 1 1 vrp1
} } */
-/* { dg-final { scan-tree-dump-times Folding predicate i_.*to 0 1 vrp1
} } */
+/* { dg-final { scan-tree-dump-times Folding predicate i_.*0 to 0 1
vrp1 } } */
+/* { dg-final { scan-tree-dump-times Folding predicate j_.*0 to 1 1
vrp1 } } */
+/* { dg-final { scan-tree-dump-times Folding predicate i_.*j_.* to 0 1
vrp1 { xfail *-*-* } } } */
/* { dg-final { cleanup-tree-dump vrp1 } } */
Re: Minor testsuite improvement
Hello! This patch tightens the expected output from the vrp dump which has the side effect of making each test's string unique. Obviously the masked failure is xfailed. OK for the trunk? Hmm, but if the SSA versions are simply i_10 then i_.*0 will still match it the same? I think you want the more elaborate 'i_\[0-9\]* . 0' here? IMO 'i_\[0-9\]+' would be better, we need at least one digit here. Uros.
Re: [PATCH] Fix PR55882
On Thu, 10 Jan 2013, Richard Biener wrote:
On Thu, 10 Jan 2013, Richard Biener wrote:
On Wed, 9 Jan 2013, Eric Botcazou wrote:
This fixes PR55882 - set_mem_attributes_minus_bitpos misses to
account for the to-be applied bitpos when computing MEM_ALIGN.
It extracts alignment from 't' instead of t - bitpos.
Bootstrapped and tested on x86_64-unknown-linux-gnu, bootstrap
and regtest running on mips.
Does this look sensible?
I'm not sure, bitpos isn't taken into account in the other cases when the
alignment is computed. adjust_address_1 is supposed to properly adjust
the
alignment by the time the offset is applied.
Yes, but alignment adjustment assumes the alignment is that of
the actual MEM, not that of the MEM_EXPR. Thus, MEM_ALIGN is the
alignment of MEM_EXPR - MEM_OFFSET.
It's true that the other paths setting align_computed have the
same issue. I can produce a followup (or preparatory patch)
that at least removes the code that is redundant. For example
/* If this is a decl, set the attributes of the MEM from it. */
if (DECL_P (t))
{
attrs.expr = t;
attrs.offset_known_p = true;
attrs.offset = 0;
apply_bitpos = bitpos;
new_size = DECL_SIZE_UNIT (t);
attrs.align = DECL_ALIGN (t);
align_computed = true;
}
the alignment computation can be handled by the get_object_alignment
path just fine, no need to duplicate it here, likewise for the
CONSTANT_CLASS_P case. The ARRAY_REF path is scary enough to
me (I'd rather drop it completely ... it's probably designed to
make more cases covered by nonoverlapping_component_refs_p?).
At least handling of DECL/COMPONENT_REF below the ARRAY_REFs
should be commonized - for example the COMPONENT_REF case
else if (TREE_CODE (t2) == COMPONENT_REF)
{
attrs.expr = t2;
attrs.offset_known_p = false;
if (host_integerp (off_tree, 1))
{
attrs.offset_known_p = true;
attrs.offset = tree_low_cst (off_tree, 1);
apply_bitpos = bitpos;
}
doesn't adjust 't' nor does it compute alignment. So it
clearly cannot be that MEM_ALIGN is supposed to be the
alignment of MEM_EXPR.
That is - the question boils down to _what_ should MEM_ALIGN
be the alignment of? In all the rest of the compiler it surely
is the alignment of XEXP (mem, 0) - to make it differ from that
during a brief period of expand_assignment sounds fragile at least.
I'll try to simplify code paths without changing behavior first.
Going over this what's strange as well is that we adjust MEM_SIZE
with bitpos, too. At least when the MEM has non-BLKmode this
means that MEMs mode and MEM_SIZE is inconsistent? Or how do
a MEMs mode and MEM_SIZE relate?
I came up with the following patch that shouldn't change
behavior but which makes it easier to follow what happens
(it will also make bitpos handling with my alignment patch fix
consistent).
The patch moves attrs.align precompute into the !TYPE_P path
and for the DECL / CONSTANT align compute use get_object_alignment
which DTRT for it (it's still buggy because we later will use
MAX of this alignment and the correct alignment - but I didn't
want to change behavior here).
The patch removes explicit align computation on the DECL_P
and CONSTANT_CLASS_P paths because that's handled by
the get_object_alignment path in the same way. That also
allows the ARRAY_REF base paths to be simplified and commonized
(the ARRAY_REF and then DECL_P path will get better align
computation this way, get_object_alignment has more elaborate
off_tree handling).
As followup I'd remove the MEM_REF/TARGET_MEM_REF alignment
precompute completely - it's buggy (due to the later use of MAX).
That is, consider the below as explanatory intermediate step,
any real patch would get rid of that precompute immediately
(and then probably not re-use anything we got from default or
pre-existing MEM_ATTRs but simply overwrite attrs.align.
See 2nd patch below.
Which needs the bitpos accounting for fix as otherwise
get_object_alignment returns bit-alignment.
And I believe we unconditionally need to account for bitpos,
not only apply_bitpos (which is for MEM_OFFSET and irrelevant
if MEM_EXPR is not set).
Which makes me arrive at the following fix for PR55882
(Martin has completed bootstrap regtest on SPARC for the
original but agreed incomplete fix). It's not backportable
to 4.7 this way (the original fix would have worked, even
when slightly altering it to cover also the align_computed
cases).
So - do we want to go this full way or a more conservative
way also applicable literally to 4.7 (conservative in, leaving
some wrong-align holes open)?
Thanks,
Richard.
2013-01-10
Re: libsanitizer mege from upstream r171973
On Thu, Jan 10, 2013 at 11:07:26AM +0400, Konstantin Serebryany wrote: Our internal LLVM bots (Linux, Mac and Android) are also green, but since the changes are large something may potentially break on other platforms. Ok to commit? Ok, but it would be nice if the prctl stuff in asan_test.cc could be made more robust. Not all Linux kernels support prctl (PR_SET_NAME, ...) (only 2.6.9+), and not all prctl.h headers define PR_SET_NAME. So, I'd guard the prctl call with additional #ifdef PR_SET_NAME #endif guards, and set some bool variable if the prctl failed (or if it wasn't #called at all), and if that global bool variable was true, used a different expected error message (.* in the place of the expected error string). Or perhaps just have TryToSetThreadName return whether it succeeded or not, also call TryToSetThreadName in ThreadNamesTest directly and just return it it failed. BTW, I had a look at your qsort overflow testcase, and I get: $ ./xg++ -B ./ -O2 -fsanitize=address -o overflow-in-qsort overflow-in-qsort.cc -B ../x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs/ -B ../x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/ -Wl,-rpath,../x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/ -fno-omit-frame-pointer -g $ ASAN_OPTIONS=fast_unwind_on_fatal=1 ./overflow-in-qsort 21 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py = ==24758== ERROR: AddressSanitizer: global-buffer-overflow on address 0x00601448 at pc 0x400acf bp 0x7fffb9caee90 sp 0x7fffb9caee88 WRITE of size 4 at 0x00601448 thread T0 #0 0x400ace in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19 #1 0x3216c37bb1 in ?? ??:0 0x00601448 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x601420) of size 40 ... while with $ ASAN_OPTIONS=fast_unwind_on_fatal=0 ./overflow-in-qsort 21 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py = ==24763== ERROR: AddressSanitizer: global-buffer-overflow on address 0x00601448 at pc 0x400acf bp 0x772db050 sp 0x772db048 WRITE of size 4 at 0x00601448 thread T0 #0 0x400ace in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19 #1 0x3216c37bb1 in ?? ??:0 #2 0x3216c3809b in ?? ??:0 #3 0x400b1b in MyQsort /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:26 #4 0x4008c9 in main /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:33 #5 0x3216c21734 in ?? ??:0 #6 0x400944 in _start ??:0 0x00601448 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x601420) of size 40 ... which is much better (Fedora 17 x86_64). Similarly for 32-bit: $ ./xg++ -B ./ -O2 -fsanitize=address -o overflow-in-qsort overflow-in-qsort.cc -B ../x86_64-unknown-linux-gnu/32/libstdc++-v3/src/.libs/ -B ../x86_64-unknown-linux-gnu/32/libsanitizer/asan/.libs/ -Wl,-rpath,../x86_64-unknown-linux-gnu/32/libsanitizer/asan/.libs/ -fno-omit-frame-pointer -g -m32 $ ASAN_OPTIONS=fast_unwind_on_fatal=1 ./overflow-in-qsort 21 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py = ==24774== ERROR: AddressSanitizer: global-buffer-overflow on address 0x08049fc8 at pc 0x80488fe bp 0xffd03548 sp 0xffd0353c WRITE of size 4 at 0x08049fc8 thread T0 #0 0x80488fd in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19 #1 0x4e00fcb2 in ?? ??:0 addr2line: '': No such file #2 0x27ff in 0x08049fc8 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x8049fa0) of size 40 ... vs. ASAN_OPTIONS=fast_unwind_on_fatal=0 ./overflow-in-qsort 21 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py = ==24780== ERROR: AddressSanitizer: global-buffer-overflow on address 0x08049fc8 at pc 0x80488fe bp 0xffbfcf88 sp 0xffbfcf7c WRITE of size 4 at 0x08049fc8 thread T0 #0 0x80488fd in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19 #1 0x4e00fcb2 in ?? ??:0 #2 0x4e010206 in ?? ??:0 #3 0x4e0102ee in ?? ??:0 #4 0x8048955 in MyQsort /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:26 #5 0x804870c in main /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:33 #6 0x4dff8634 in ?? ??:0 #7 0x8048794 in _start ??:0 0x08049fc8 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x8049fa0) of size 40 Clearly asan_symbolize.py doesn't cope with debug info stripped to separate file (I have glibc-debuginfo installed), but AFAIK iant's libbacktrace doesn't yet either (nor understand dwz DWARF extensions, but Fedora 17 still doesn't use them, only Fedora 18+ does). Anyway, I've yet to see a testcase where the fast unwinder results in better backtrace with default
Re: [RFC, x86] Changes for AVX and AVX2 processors
Hello Uros, It seems I didn't sent a patch with last changes. Sorry if so. Vladimir 2012-12-27 Vladimir Yakovlev vladimir.b.yakov...@intel.com * config/i386/i386-c.c (ix86_target_macros_internal): New case. (ix86_target_macros_internal): Likewise. * config/i386/i386.c (m_CORE2I7): Removed. (m_CORE_HASWELL): New macro. (m_CORE_ALL): Likewise. (initial_ix86_tune_features): m_CORE2I7 is replaced by m_CORE_ALL. (initial_ix86_arch_features): Likewise. (processor_target_table): Initializations for Core avx2. (cpu_names): New names core-avx2. (ix86_option_override_internal): Changed PROCESSOR_COREI7 by PROCESSOR_CORE_HASWELL. (ix86_issue_rate): New case. (ia32_multipass_dfa_lookahead): Likewise. (ix86_sched_init_global): Likewise. * config/i386/i386.h (TARGET_HASWELL): New macro. (target_cpu_default): New TARGET_CPU_DEFAULT_haswell. (processor_type): New PROCESSOR_HASWELL. 2012/12/30 Uros Bizjak ubiz...@gmail.com: On Sun, Dec 30, 2012 at 5:05 PM, Vladimir Yakovlev vbyakov...@gmail.com wrote: I fixed typos and added CalangeLog. 2012-12-27 Vladimir Yakovlev vladimir.b.yakov...@intel.com * config/i386/i386-c.c (ix86_target_macros_internal): New case. (ix86_target_macros_internal): Likewise. * config/i386/i386.c (m_CORE2I7): Removed. (m_CORE_HASWELL): New macro. (m_CORE_ALL): Likewise. (initial_ix86_tune_features): m_CORE2I7 is replaced by m_CORE_ALL. (initial_ix86_arch_features): Likewise. (processor_target_table): Initializations for Core avx2. (cpu_names): New names core-avx2. (ix86_option_override_internal): Changed PROCESSOR_COREI7 by PROCESSOR_CORE_HASWELL. (ix86_issue_rate): New case. (ia32_multipass_dfa_lookahead): Likewise. (ix86_sched_init_global): Likewise. (get_builtin_code_for_version): Likewise. * config/i386/i386.h (TARGET_HASWELL): New macro. (target_cpu_default): New TARGET_CPU_DEFAULT_haswell. (processor_type): New PROCESSOR_HASWELL. Please remove this part, it should be part of processor dispatcher part: @@ -28705,6 +28712,10 @@ get_builtin_code_for_version (tree decl, tree *predicate_list) arg_str = corei7; priority = P_PROC_SSE4_2; break; + case PROCESSOR_HASWELL: + arg_str = core-avx2; + priority = P_PROC_SSE4_2; + break; case PROCESSOR_ATOM: arg_str = atom; priority = P_PROC_SSSE3; Uros. patch1 Description: Binary data
Re: libsanitizer mege from upstream r171973
On Thu, Jan 10, 2013 at 2:59 PM, Jakub Jelinek ja...@redhat.com wrote: On Thu, Jan 10, 2013 at 11:07:26AM +0400, Konstantin Serebryany wrote: Our internal LLVM bots (Linux, Mac and Android) are also green, but since the changes are large something may potentially break on other platforms. Ok to commit? Ok, but it would be nice if the prctl stuff in asan_test.cc could be made more robust. Not all Linux kernels support prctl (PR_SET_NAME, ...) (only 2.6.9+), and not all prctl.h headers define PR_SET_NAME. So, I'd guard the prctl call with additional #ifdef PR_SET_NAME Ok. I'd prefer to guard the whole test with #if defined(__linux__) defined(PR_SET_NAME). #endif guards, and set some bool variable if the prctl failed (or if it wasn't #called at all), and if that global bool variable was true, used a different expected error message (.* in the place of the expected error string). Or perhaps just have TryToSetThreadName return whether it succeeded or not, also call TryToSetThreadName in ThreadNamesTest directly and just return it it failed. BTW, I had a look at your qsort overflow testcase, and I get: $ ./xg++ -B ./ -O2 -fsanitize=address -o overflow-in-qsort overflow-in-qsort.cc -B ../x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs/ -B ../x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/ -Wl,-rpath,../x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/ -fno-omit-frame-pointer -g $ ASAN_OPTIONS=fast_unwind_on_fatal=1 ./overflow-in-qsort 21 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py = ==24758== ERROR: AddressSanitizer: global-buffer-overflow on address 0x00601448 at pc 0x400acf bp 0x7fffb9caee90 sp 0x7fffb9caee88 WRITE of size 4 at 0x00601448 thread T0 #0 0x400ace in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19 #1 0x3216c37bb1 in ?? ??:0 0x00601448 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x601420) of size 40 ... while with $ ASAN_OPTIONS=fast_unwind_on_fatal=0 ./overflow-in-qsort 21 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py = ==24763== ERROR: AddressSanitizer: global-buffer-overflow on address 0x00601448 at pc 0x400acf bp 0x772db050 sp 0x772db048 WRITE of size 4 at 0x00601448 thread T0 #0 0x400ace in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19 #1 0x3216c37bb1 in ?? ??:0 #2 0x3216c3809b in ?? ??:0 #3 0x400b1b in MyQsort /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:26 #4 0x4008c9 in main /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:33 #5 0x3216c21734 in ?? ??:0 #6 0x400944 in _start ??:0 0x00601448 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x601420) of size 40 ... which is much better (Fedora 17 x86_64). Good! Similarly for 32-bit: $ ./xg++ -B ./ -O2 -fsanitize=address -o overflow-in-qsort overflow-in-qsort.cc -B ../x86_64-unknown-linux-gnu/32/libstdc++-v3/src/.libs/ -B ../x86_64-unknown-linux-gnu/32/libsanitizer/asan/.libs/ -Wl,-rpath,../x86_64-unknown-linux-gnu/32/libsanitizer/asan/.libs/ -fno-omit-frame-pointer -g -m32 $ ASAN_OPTIONS=fast_unwind_on_fatal=1 ./overflow-in-qsort 21 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py = ==24774== ERROR: AddressSanitizer: global-buffer-overflow on address 0x08049fc8 at pc 0x80488fe bp 0xffd03548 sp 0xffd0353c WRITE of size 4 at 0x08049fc8 thread T0 #0 0x80488fd in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19 #1 0x4e00fcb2 in ?? ??:0 addr2line: '': No such file #2 0x27ff in 0x08049fc8 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x8049fa0) of size 40 ... vs. ASAN_OPTIONS=fast_unwind_on_fatal=0 ./overflow-in-qsort 21 | /usr/src/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py = ==24780== ERROR: AddressSanitizer: global-buffer-overflow on address 0x08049fc8 at pc 0x80488fe bp 0xffbfcf88 sp 0xffbfcf7c WRITE of size 4 at 0x08049fc8 thread T0 #0 0x80488fd in QsortCallback /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:19 #1 0x4e00fcb2 in ?? ??:0 #2 0x4e010206 in ?? ??:0 #3 0x4e0102ee in ?? ??:0 #4 0x8048955 in MyQsort /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:26 #5 0x804870c in main /usr/src/gcc/obj2/gcc/overflow-in-qsort.cc:33 #6 0x4dff8634 in ?? ??:0 #7 0x8048794 in _start ??:0 0x08049fc8 is located 0 bytes to the right of global variable 'global_array (overflow-in-qsort.cc)' (0x8049fa0) of size 40 Clearly asan_symbolize.py doesn't cope with debug info stripped to separate file (I have glibc-debuginfo
Re: [RFC, x86] Changes for AVX and AVX2 processors
On Thu, Jan 10, 2013 at 12:12 PM, Vladimir Yakovlev vbyakov...@gmail.com wrote: It seems I didn't sent a patch with last changes. Sorry if so. Vladimir 2012-12-27 Vladimir Yakovlev vladimir.b.yakov...@intel.com * config/i386/i386-c.c (ix86_target_macros_internal): New case. (ix86_target_macros_internal): Likewise. * config/i386/i386.c (m_CORE2I7): Removed. (m_CORE_HASWELL): New macro. (m_CORE_ALL): Likewise. (initial_ix86_tune_features): m_CORE2I7 is replaced by m_CORE_ALL. (initial_ix86_arch_features): Likewise. (processor_target_table): Initializations for Core avx2. (cpu_names): New names core-avx2. (ix86_option_override_internal): Changed PROCESSOR_COREI7 by PROCESSOR_CORE_HASWELL. (ix86_issue_rate): New case. (ia32_multipass_dfa_lookahead): Likewise. (ix86_sched_init_global): Likewise. * config/i386/i386.h (TARGET_HASWELL): New macro. (target_cpu_default): New TARGET_CPU_DEFAULT_haswell. (processor_type): New PROCESSOR_HASWELL. As strictly tuning patch, the patch is OK for mainline. Please note, that (eventual) processor dispatcher patch will have to wait for next stage1. Thanks, Uros.
Re: libsanitizer mege from upstream r171973
On Thu, Jan 10, 2013 at 03:27:41PM +0400, Konstantin Serebryany wrote: So, I'd guard the prctl call with additional #ifdef PR_SET_NAME Ok. I'd prefer to guard the whole test with #if defined(__linux__) defined(PR_SET_NAME). That is not sufficient. You can have PR_SET_NAME defined in the headers, but still the underlying kernel doesn't need to handle it. It is very likely that I will flip fast_unwind_on_fatal to 0 next week. Perfect, thanks. Jakub
Re: [RFC, x86] Changes for AVX and AVX2 processors
On Thu, Jan 10, 2013 at 12:28:24PM +0100, Uros Bizjak wrote: On Thu, Jan 10, 2013 at 12:12 PM, Vladimir Yakovlev vbyakov...@gmail.com wrote: It seems I didn't sent a patch with last changes. Sorry if so. Vladimir 2012-12-27 Vladimir Yakovlev vladimir.b.yakov...@intel.com Missing at the end of line. * config/i386/i386-c.c (ix86_target_macros_internal): New case. (ix86_target_macros_internal): Likewise. There is some additional space at the beginning of this line (note, all ChangeLog lines but the one with date should be tab indented, not space). Jakub
Re: [PATCH] Fix PR55882
Going over this what's strange as well is that we adjust MEM_SIZE with bitpos, too. At least when the MEM has non-BLKmode this means that MEMs mode and MEM_SIZE is inconsistent? Or how do a MEMs mode and MEM_SIZE relate? Yes, the MEM attributes are incomplete/inconsistent between the call to set_mem_attributes_minus_bitpos and the subsequent adjustment by bitpos. That's why only the final result should matter and need be correct. Let me try to forge my own opinion on the PR before answering further. -- Eric Botcazou
Re: libsanitizer mege from upstream r171973
On Thu, Jan 10, 2013 at 03:57:44PM +0400, Konstantin Serebryany wrote: That is not sufficient. You can have PR_SET_NAME defined in the headers, but still the underlying kernel doesn't need to handle it. Like this? http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/tests/asan_test.cc?r1=172048r2=172059pathrev=172059diff_format=h Yeah. I wouldn't create a separate thread just to call prctl, I believe it should work even in the initial thread, doesn't it? But perhaps you don't want to affect the name of the original testcase when running under gtest native. Jakub
[PATCH] Fix parts of PR55792
I think that PR55792 shows we still have locations in our IL
with BLOCKs that are not in the functions BLOCK tree which
will lead to hard to trigger GC issues with location/BLOCKs
merged into location_t now.
The following fixes one issue I noticed when writing a verifier
for locations. remove_unused_scope_blocks does not consider
virtual PHIs when keeping BLOCKs live - which is IMHO sensible.
Thus the following patch makes sure we do not set locations
of virtual PHI args.
Bootstrap and regtest ongoing on x86_64-unknown-linux-gnu
(with the checker enabled ... which I'll post separately).
Thanks,
Richard.
2013-01-10 Richard Biener rguent...@suse.de
PR bootstrap/55792
* tree-into-ssa.c (rewrite_add_phi_arguments): Do not set
locations for virtual PHI arguments.
(rewrite_update_phi_arguments): Likewise.
Index: gcc/tree-into-ssa.c
===
*** gcc/tree-into-ssa.c (revision 195079)
--- gcc/tree-into-ssa.c (working copy)
*** rewrite_add_phi_arguments (basic_block b
*** 1355,1367
for (gsi = gsi_start_phis (e-dest); !gsi_end_p (gsi);
gsi_next (gsi))
{
! tree currdef;
! gimple stmt;
phi = gsi_stmt (gsi);
! currdef = get_reaching_def (SSA_NAME_VAR (gimple_phi_result (phi)));
! stmt = SSA_NAME_DEF_STMT (currdef);
! add_phi_arg (phi, currdef, e, gimple_location (stmt));
}
}
}
--- 1355,1372
for (gsi = gsi_start_phis (e-dest); !gsi_end_p (gsi);
gsi_next (gsi))
{
! tree currdef, res;
! location_t loc;
phi = gsi_stmt (gsi);
! res = gimple_phi_result (phi);
! currdef = get_reaching_def (SSA_NAME_VAR (res));
! /* Virtual operand PHI args do not need a location. */
! if (virtual_operand_p (res))
! loc = UNKNOWN_LOCATION;
! else
! loc = gimple_location (SSA_NAME_DEF_STMT (currdef));
! add_phi_arg (phi, currdef, e, loc);
}
}
}
*** rewrite_update_phi_arguments (basic_bloc
*** 2018,2037
/* Update the argument if there is a reaching def. */
if (reaching_def)
{
- gimple stmt;
source_location locus;
int arg_i = PHI_ARG_INDEX_FROM_USE (arg_p);
SET_USE (arg_p, reaching_def);
- stmt = SSA_NAME_DEF_STMT (reaching_def);
! /* Single element PHI nodes behave like copies, so get the
!location from the phi argument. */
! if (gimple_code (stmt) == GIMPLE_PHI
! gimple_phi_num_args (stmt) == 1)
! locus = gimple_phi_arg_location (stmt, 0);
else
! locus = gimple_location (stmt);
gimple_phi_arg_set_location (phi, arg_i, locus);
}
--- 2023,2048
/* Update the argument if there is a reaching def. */
if (reaching_def)
{
source_location locus;
int arg_i = PHI_ARG_INDEX_FROM_USE (arg_p);
SET_USE (arg_p, reaching_def);
! /* Virtual operands do not need a location. */
! if (virtual_operand_p (reaching_def))
! locus = UNKNOWN_LOCATION;
else
! {
! gimple stmt = SSA_NAME_DEF_STMT (reaching_def);
!
! /* Single element PHI nodes behave like copies, so get the
!location from the phi argument. */
! if (gimple_code (stmt) == GIMPLE_PHI
! gimple_phi_num_args (stmt) == 1)
! locus = gimple_phi_arg_location (stmt, 0);
! else
! locus = gimple_location (stmt);
! }
gimple_phi_arg_set_location (phi, arg_i, locus);
}
Re: libsanitizer mege from upstream r171973
On Thu, Jan 10, 2013 at 4:23 PM, Jakub Jelinek ja...@redhat.com wrote: On Thu, Jan 10, 2013 at 03:57:44PM +0400, Konstantin Serebryany wrote: That is not sufficient. You can have PR_SET_NAME defined in the headers, but still the underlying kernel doesn't need to handle it. Like this? http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/tests/asan_test.cc?r1=172048r2=172059pathrev=172059diff_format=h Yeah. I wouldn't create a separate thread just to call prctl, I believe it should work even in the initial thread, doesn't it? But perhaps you don't want to affect the name of the original testcase when running under gtest native. Exactly. Thanks! --kcc Jakub
Re: [PATCH] Fix PR55882
Hi, On Thu, 10 Jan 2013, Eric Botcazou wrote: Going over this what's strange as well is that we adjust MEM_SIZE with bitpos, too. At least when the MEM has non-BLKmode this means that MEMs mode and MEM_SIZE is inconsistent? Or how do a MEMs mode and MEM_SIZE relate? Yes, the MEM attributes are incomplete/inconsistent between the call to set_mem_attributes_minus_bitpos and the subsequent adjustment by bitpos. The problem with this intermediate inconsistency is that ... That's why only the final result should matter and need be correct. ... this can't be ensured anymore. In some cases the inconsistency between the mem attributes and what XEXP(MEM, 0) represents can't be repaired by the later bitpos adjustments, or better it can be only be repaired by falling back to the conservative side for e.g. the alignment, because we don't store enough information in the mem attributes to recover what was lost. When briefly discussing this yesterday I suggested (without having the code in front of me) that it be best to simply set the mem attributes only at the very end, after having computed to final MEM address including all adjustments, instead of generating wrong mem attributes initially and hoping for the adjustments to make it come out right at the end. Ciao, Michael.
[testsuite] Adding -fno-pic to certain tests
Hi, It all begun with discussion here http://gcc.gnu.org/ml/gcc/2012-11/msg00205.html Since -fpic option is turned on by default in Android we have certain test fails. The reason for that is that those tests rely on the availability of functions, defined in them and with -fpic compiler conservatively assumes that they are AVAIL_OVERWRITABLE. The attached patch adding -fno-pic option for tests that fail because of that. 2013-01-10 Alexander Ivchenko alexander.ivche...@intel.com * g++.dg/ipa/ivinline-1.C: Add -fno-pic option. * g++.dg/ipa/ivinline-2.C: Likewise. * g++.dg/ipa/ivinline-3.C: Likewise. * g++.dg/ipa/ivinline-4.C: Likewise. * g++.dg/ipa/ivinline-5.C: Likewise. * g++.dg/ipa/ivinline-7.C: Likewise. * g++.dg/ipa/ivinline-8.C: Likewise. * g++.dg/ipa/ivinline-9.C: Likewise. * g++.dg/cpp0x/noexcept03.C: Likewise. * gcc.dg/const-1.c: Likewise. * gcc.dg/ipa/pure-const-1.c: Likewise. * gcc.dg/noreturn-8.c: Likewise. * gcc.dg/tree-ssa/ipa-split-5.c: Likewise. * gcc.dg/tree-ssa/loadpre6.c: Likewise. * gcc.c-torture/execute/pr33992.c: Likewise. * gcc.c-torture/execute/pr33992.x: New file. ok for mainline? thanks, Alexander adding_fno_pic.patch Description: Binary data
Re: [PATCH] Fix PR55882
On Thu, 10 Jan 2013, Michael Matz wrote:
Hi,
On Thu, 10 Jan 2013, Eric Botcazou wrote:
Going over this what's strange as well is that we adjust MEM_SIZE
with bitpos, too. At least when the MEM has non-BLKmode this
means that MEMs mode and MEM_SIZE is inconsistent? Or how do
a MEMs mode and MEM_SIZE relate?
Yes, the MEM attributes are incomplete/inconsistent between the call to
set_mem_attributes_minus_bitpos and the subsequent adjustment by bitpos.
The problem with this intermediate inconsistency is that ...
That's why only the final result should matter and need be correct.
... this can't be ensured anymore. In some cases the inconsistency
between the mem attributes and what XEXP(MEM, 0) represents can't be
repaired by the later bitpos adjustments, or better it can be only be
repaired by falling back to the conservative side for e.g. the alignment,
because we don't store enough information in the mem attributes to recover
what was lost.
When briefly discussing this yesterday I suggested (without having the
code in front of me) that it be best to simply set the mem attributes only
at the very end, after having computed to final MEM address including all
adjustments, instead of generating wrong mem attributes initially and
hoping for the adjustments to make it come out right at the end.
Btw, if the expand_assignment code is really special we should move
minus-bitpos handling to its sole caller like with the following
(_minus_bitpos variant to be unified with set_mem_attributes and
set_mem_attributes_1 to be introduced that just computes new mem
attributes so that expand_assignment can modify them in a more
cheap way and call set_mem_attrs itself).
Richard.
Index: gcc/expr.c
===
--- gcc/expr.c (revision 195083)
+++ gcc/expr.c (working copy)
@@ -4856,7 +4856,16 @@ expand_assignment (tree to, tree from, b
DECL_RTX of the parent struct. Don't munge it. */
to_rtx = shallow_copy_rtx (to_rtx);
- set_mem_attributes_minus_bitpos (to_rtx, to, 0, bitpos);
+ set_mem_attributes_minus_bitpos (to_rtx, to, 0, 0);
+ if (bitpos / BITS_PER_UNIT != 0)
+ {
+ if (MEM_OFFSET_KNOWN_P (to_rtx))
+ set_mem_offset (to_rtx,
+ MEM_OFFSET (to_rtx) - bitpos /
BITS_PER_UNIT);
+ if (MEM_SIZE_KNOWN_P (to_rtx))
+ set_mem_size (to_rtx,
+ MEM_SIZE (to_rtx) + bitpos / BITS_PER_UNIT);
+ }
/* Deal with volatile and readonly fields. The former is only
done for MEM. Also set MEM_KEEP_ALIAS_SET_P if needed. */
Index: gcc/emit-rtl.c
===
--- gcc/emit-rtl.c (revision 195083)
+++ gcc/emit-rtl.c (working copy)
@@ -1566,9 +1566,8 @@ get_mem_align_offset (rtx mem, unsigned
void
set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
-HOST_WIDE_INT bitpos)
+HOST_WIDE_INT)
{
- HOST_WIDE_INT apply_bitpos = 0;
tree type;
struct mem_attrs attrs, *defattrs, *refattrs;
addr_space_t as;
@@ -1736,7 +1735,6 @@ set_mem_attributes_minus_bitpos (rtx ref
attrs.expr = t;
attrs.offset_known_p = true;
attrs.offset = 0;
- apply_bitpos = bitpos;
new_size = DECL_SIZE_UNIT (t);
attrs.align = DECL_ALIGN (t);
align_computed = true;
@@ -1758,7 +1756,6 @@ set_mem_attributes_minus_bitpos (rtx ref
attrs.expr = t;
attrs.offset_known_p = true;
attrs.offset = 0;
- apply_bitpos = bitpos;
if (DECL_BIT_FIELD (TREE_OPERAND (t, 1)))
new_size = DECL_SIZE_UNIT (TREE_OPERAND (t, 1));
}
@@ -1809,7 +1806,6 @@ set_mem_attributes_minus_bitpos (rtx ref
align_computed = true;
attrs.offset_known_p = true;
attrs.offset = ioff;
- apply_bitpos = bitpos;
}
}
else if (TREE_CODE (t2) == COMPONENT_REF)
@@ -1820,7 +1816,6 @@ set_mem_attributes_minus_bitpos (rtx ref
{
attrs.offset_known_p = true;
attrs.offset = tree_low_cst (off_tree, 1);
- apply_bitpos = bitpos;
}
/* ??? Any reason the field size would be different than
the size we got from the type? */
@@ -1834,7 +1829,6 @@ set_mem_attributes_minus_bitpos (rtx ref
attrs.expr = t;
attrs.offset_known_p = true;
attrs.offset = 0;
- apply_bitpos = bitpos;
}
if (!align_computed)
@@ -1852,17 +1846,6 @@ set_mem_attributes_minus_bitpos (rtx ref
attrs.size = tree_low_cst (new_size, 1);
}
- /* If
Re: [testsuite] Fix g++.dg/tls/thread_local* on Solaris 9
Rainer Orth r...@cebitec.uni-bielefeld.de writes: 2013-01-08 Rainer Orth r...@cebitec.uni-bielefeld.de * g++.dg/tls/thread_local-cse.C: Don't xfail on *-*-solaris2.9. Add tls options. * g++.dg/tls/thread_local2.C: Likewise. * g++.dg/tls/thread_local2g.C: Likewise. * g++.dg/tls/thread_local6.C: Likewise. * g++.dg/tls/thread_local-order1.C: Add tls options. * g++.dg/tls/thread_local-order2.C: Likewise. * g++.dg/tls/thread_local3.C: Likewise. * g++.dg/tls/thread_local3g.C: Likewise. * g++.dg/tls/thread_local4.C: Likewise. * g++.dg/tls/thread_local4g.C: Likewise. * g++.dg/tls/thread_local5.C: Likewise. * g++.dg/tls/thread_local5g.C: Likewise. * g++.dg/tls/thread_local6g.C: Likewise. Given that there were no objections and the patch falls completely under my Solaris and testsuite maintainerships, I've installed it. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
[PATCH] Call set_mem_attributes instead of set_mem_attributes_minus_bitpos where possible
Committed as obvious (leaves a single caller of set_mem_attributes_minus_bitpos). Richard. 2013-01-10 Richard Biener rguent...@suse.de * builtins.c (expand_builtin_init_trampoline): Use set_mem_attributes. Index: gcc/builtins.c === --- gcc/builtins.c (revision 195083) +++ gcc/builtins.c (working copy) @@ -4853,8 +4853,7 @@ expand_builtin_init_trampoline (tree exp within the local function's FRAME decl. Either way, let's see if we can fill in the MEM_ATTRs for this memory. */ if (TREE_CODE (t_tramp) == ADDR_EXPR) -set_mem_attributes_minus_bitpos (m_tramp, TREE_OPERAND (t_tramp, 0), -true, 0); +set_mem_attributes (m_tramp, TREE_OPERAND (t_tramp, 0), true); /* Creator of a heap trampoline is responsible for making sure the address is aligned to at least STACK_BOUNDARY. Normally malloc
Re: [testsuite] Adding -fno-pic to certain tests
On Thu, Jan 10, 2013 at 2:50 PM, Alexander Ivchenko aivch...@gmail.com wrote: Hi, It all begun with discussion here http://gcc.gnu.org/ml/gcc/2012-11/msg00205.html Since -fpic option is turned on by default in Android we have certain test fails. The reason for that is that those tests rely on the availability of functions, defined in them and with -fpic compiler conservatively assumes that they are AVAIL_OVERWRITABLE. The attached patch adding -fno-pic option for tests that fail because of that. I think this should be a dg-requires nopic instead. Otherwise testing with -fPIC/-fno-pic will not show expected differences. Richard. 2013-01-10 Alexander Ivchenko alexander.ivche...@intel.com * g++.dg/ipa/ivinline-1.C: Add -fno-pic option. * g++.dg/ipa/ivinline-2.C: Likewise. * g++.dg/ipa/ivinline-3.C: Likewise. * g++.dg/ipa/ivinline-4.C: Likewise. * g++.dg/ipa/ivinline-5.C: Likewise. * g++.dg/ipa/ivinline-7.C: Likewise. * g++.dg/ipa/ivinline-8.C: Likewise. * g++.dg/ipa/ivinline-9.C: Likewise. * g++.dg/cpp0x/noexcept03.C: Likewise. * gcc.dg/const-1.c: Likewise. * gcc.dg/ipa/pure-const-1.c: Likewise. * gcc.dg/noreturn-8.c: Likewise. * gcc.dg/tree-ssa/ipa-split-5.c: Likewise. * gcc.dg/tree-ssa/loadpre6.c: Likewise. * gcc.c-torture/execute/pr33992.c: Likewise. * gcc.c-torture/execute/pr33992.x: New file. ok for mainline? thanks, Alexander
Re: [committed] 2011 and 2012 Copyright year updates
On Tue, Jan 08, 2013 at 08:48:52PM +, Richard Sandiford wrote: Tested on x86_64-linux-gnu. OK to install? Ok, thanks. Jakub
[Patch, fortran] PR53537 Explicit import of USE renamed symbol.
Hello,
for the case:
[...]
use select_precision, only: wp = dp
interface
subroutine ode_derivative(x)
import :: wp
[...]
`wp' is currently imported in the subroutine namespace under its
original name `dp', which leads to an error if one tries to use `wp'.
The core of the fix, which is basically a collection of the patches
Tobias posted in the PR, uses the matched name instead of the original
name for the symtree in the subroutine namespace.
Tobias' patches regress on import7.f90 because variable_decl lookups for
the type in the interface namespace (to check that it has been declared)
using the original name (which is the only one available there).
The fix I propose for that is to remove the regressing error, and try to
trigger the existing generic code diagnosing undeclared types. The
latter doesn't trigger currently because gfc_get_ha_symtree keep going
up the parent namespace until it finds a symbol, so in an interface
block, it is guaranteed to find a declared symbol, even if the latter
hasn't been imported in the interface.
The fix for that checks whether we are in an interface body without
blank import statement, and aborts the lookup in that case. A few
adjustments are then needed where the parent namespace is accessed
directly, thus bypassing the above check.
The change from the interface-specific error to the more general error
needs a few test cases to be adjusted, namely import2.f90, import8.f90,
and interface_derived_type_1.f90
from:
type(fcnparms) :: fparams ! { dg-error not been declared
within the in
1
Error: The type of 'fparams' at (1) has not been declared within the
interface
to:
type(fcnparms) :: fparams ! { dg-error not been declared
within the in
1
Error: Derived type 'fcnparms' at (1) is being used before it is defined
The caret is slightly better, the message is slightly worse. I think
it's OK, but could consider trying to issue a better error message.
Otherwise it passes the test suite. OK for trunk?
As the code impacts the name to symbol resolution code, it has a big
potential for breakage; the bug is a regression however, so I plan to
backport to 4.7 and 4.6, say, two weeks after trunk at least (if I don't
forget). Does it sound good?
Mikael
fortran/
2013-01-10 Tobias Burnus bur...@net-b.de
Mikael Morin mik...@gcc.gnu.org
PR fortran/53537
* symbol.c (gfc_find_sym_tree): Don't look for the symbol outside an
interface block.
(gfc_get_ha_symtree): Let gfc_find_sym_tree lookup the parent namespace.
* decl.c (gfc_match_data_decl): Ditto.
(variable_decl): Remove undeclared type error.
(gfc_match_import): Use renamed instead of original name.
testsuite/
2013-01-10 Tobias Burnus bur...@net-b.de
Mikael Morin mik...@gcc.gnu.org
PR fortran/53537
* gfortran.dg/import2.f90: Adjust undeclared type error message.
* gfortran.dg/import8.f90: Likewise.
* gfortran.dg/interface_derived_type_1.f90: Likewise.
* gfortran.dg/import10.f90: New test.
* gfortran.dg/import11.f90: Likewise
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 3a36cad..4af587b 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -1981,30 +1981,6 @@ variable_decl (int elem)
goto cleanup;
}
- /* An interface body specifies all of the procedure's
- characteristics and these shall be consistent with those
- specified in the procedure definition, except that the interface
- may specify a procedure that is not pure if the procedure is
- defined to be pure(12.3.2). */
- if ((current_ts.type == BT_DERIVED || current_ts.type == BT_CLASS)
- gfc_current_ns-proc_name
- gfc_current_ns-proc_name-attr.if_source == IFSRC_IFBODY
- current_ts.u.derived-ns != gfc_current_ns)
-{
- gfc_symtree *st;
- st = gfc_find_symtree (gfc_current_ns-sym_root,
current_ts.u.derived-name);
- if (!(current_ts.u.derived-attr.imported
-st != NULL
-gfc_find_dt_in_generic (st-n.sym) == current_ts.u.derived)
-!gfc_current_ns-has_import_set)
- {
- gfc_error (The type of '%s' at %C has not been declared within the
- interface, name);
- m = MATCH_ERROR;
- goto cleanup;
- }
-}
-
if (check_function_name (name) == FAILURE)
{
m = MATCH_ERROR;
@@ -3242,14 +3218,14 @@ gfc_match_import (void)
return MATCH_ERROR;
}
- if (gfc_find_symtree (gfc_current_ns-sym_root,name))
+ if (gfc_find_symtree (gfc_current_ns-sym_root, name))
{
gfc_warning ('%s' is already IMPORTed from host scoping unit
at %C., name);
goto next_item;
}
- st =
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
Gabriel Dos Reis g...@integrable-solutions.net writes:
I read your reply. I am now even more puzzled
than before. The call to uses_template_parm indicates that
we expect that code to work when are also when processing a template
(e.g. for non-dependent cases inside a template.)
That makes me wonder how it could possibly work for the
cases at hand because for non-type template arguments we need
full instantiation information to determine convertibility and
constantness.
I introduced the confusion, sorry. I overlooked the fact that this is
actually happening while calling instantiate_alias_template on Alias
with the argument the_thruthint non folded. In that code patch, we
didn't even go through convert_template_argument like what I was saying.
More on this below.
Jason Merrill ja...@redhat.com writes:
On 01/09/2013 10:02 AM, Dodji Seketeli wrote:
As the argument 'the_truthT()' we care about is type dependant,
uses_template_parms returns true and so convert_nontype_argument is
never called.
Right, but we should call it for the instantiated argument, too. We
do that for class templates by calling lookup_template_class again,
which calls coerce_template_parms. We need to make sure we call
coerce_template_parms when instantiating alias templates, too.
Indeed.
The problem is happening during the instantiation of testint using
instantiate_class_template. In that case the argument {int} was
previously properly coerced and stored in CLASSTYPE_TI_ARGS by
lookup_class_template, as you are saying. So instantiate_class_template
always uses coerced arguments.
But during the instantiation of the *members* of testint, we try to
instantiate Aliasthe_truthint, without coercing (and thus without
folding) the argument {the_truthint}. We do this using
instantiate_alias_template, called from tsubst.
The patch below makes sure instantiate_alias_template coerces the
arguments it uses to instantiate the alias template, like what I
understood you are suggesting. I have tested it without boostrap and a
full boostrap is currently running.
If this approach looks acceptable, could I replace (part of) the
template argument coercing code in lookup_class_template_1 with the new
coerce_template_parms_all_level I introduced in this patch?
Also, I am not sure if this patch would be appropriate for commit, now
that we are in 'regression-only' mode. But seeing this alias-template
related thing not working hurts me. :) So at worst I'll schedule the
patch for when stage1 opens again.
Thanks.
gcc/cp/
PR c++/55663
* pt.c (coerce_template_parms_all_levels): New static function.
(instantiate_alias_template): Use it here.
gcc/testsuite/
PR c++/55663
* g++.dg/cpp0x/alias-decl-31.C: New test.
---
gcc/cp/pt.c| 64 +-
gcc/testsuite/g++.dg/cpp0x/alias-decl-31.C | 20 ++
2 files changed, 83 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-31.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 30bafa0..aadc173 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -130,6 +130,8 @@ static tree tsubst_initializer_list (tree, tree);
static tree get_class_bindings (tree, tree, tree, tree);
static tree coerce_template_parms (tree, tree, tree, tsubst_flags_t,
bool, bool);
+static tree coerce_template_parms_all_levels (tree, tree, tree, tsubst_flags_t,
+ bool, bool);
static void tsubst_enum(tree, tree, tree);
static tree add_to_template_args (tree, tree);
static tree add_outermost_template_args (tree, tree);
@@ -6742,6 +6744,58 @@ coerce_template_parms (tree parms,
return new_inner_args;
}
+/* Like coerce_template_parms. If PARMS represents all template
+ parameters levels, this function returns a vector of vectors
+ representing all the resulting argument levels. Note that in this
+ case, only the innermost arguments are coerced because the
+ outermost ones are supposed to have been coerced already.
+
+ Otherwise, if PARMS represents only (the innermost) vectors of
+ parameters, returns a vector containing just the innermost
+ resulting arguments. */
+
+static tree
+coerce_template_parms_all_levels (tree parms,
+ tree args,
+ tree in_decl,
+ tsubst_flags_t complain,
+ bool require_all_args,
+ bool use_default_args)
+{
+ int parms_depth = TMPL_PARMS_DEPTH (parms);
+ int args_depth = TMPL_ARGS_DEPTH (args);
+ tree coerced_args;
+
+ if (parms_depth 1)
+{
+ coerced_args = make_tree_vec (parms_depth);
+ tree level;
+ int cur_depth;
+
+ for (level = parms, cur_depth = parms_depth;
+ parms_depth 0 level != NULL_TREE;
+ level = TREE_CHAIN (level), --cur_depth)
+ {
+
[PATCH][RFC] location and BLOCK checker
The following is what I use to track down PR55792 - it adds verification
that the GIMPLE IL only contains references to BLOCKs that are part
of the functions BLOCK tree. Otherwise BLOCKs lack a reference that is
seen by the garbage collector and funny things can happen.
It probably should also scan DEBUG_STMTs.
Do we want this in trunk or should it wait for stage1?
Thanks,
Richard.
Index: gcc/tree-cfg.c
===
*** gcc/tree-cfg.c.orig 2013-01-10 12:21:01.0 +0100
--- gcc/tree-cfg.c 2013-01-10 13:39:46.478923857 +0100
*** verify_eh_throw_stmt_node (void **slot,
*** 4519,4524
--- 4519,4581
return 1;
}
+ /* Verify if the location LOCs block is in BLOCKS. */
+
+ static bool
+ verify_location (pointer_set_t *blocks, location_t loc)
+ {
+ tree block = LOCATION_BLOCK (loc);
+ if (block != NULL_TREE
+!pointer_set_contains (blocks, block))
+ {
+ error (location references block not in block tree);
+ return true;
+ }
+ return false;
+ }
+
+ /* Called via walk_tree. Verify locations of expressions. */
+
+ static tree
+ verify_expr_location_1 (tree *tp, int *walk_subtrees, void *data)
+ {
+ struct pointer_set_t *blocks = (struct pointer_set_t *) data;
+
+ if (!EXPR_P (*tp))
+ {
+ *walk_subtrees = false;
+ return NULL;
+ }
+
+ location_t loc = EXPR_LOCATION (*tp);
+ if (verify_location (blocks, loc))
+ return *tp;
+
+ return NULL;
+ }
+
+ /* Called via walk_gimple_op. Verify locations of expressions. */
+
+ static tree
+ verify_expr_location (tree *tp, int *walk_subtrees, void *data)
+ {
+ struct walk_stmt_info *wi = (struct walk_stmt_info *) data;
+ return verify_expr_location_1 (tp, walk_subtrees, wi-info);
+ }
+
+ /* Insert all subblocks of BLOCK into BLOCKS and recurse. */
+
+ static void
+ collect_subblocks (pointer_set_t *blocks, tree block)
+ {
+ tree t;
+ for (t = BLOCK_SUBBLOCKS (block); t; t = BLOCK_CHAIN (t))
+ {
+ pointer_set_insert (blocks, t);
+ collect_subblocks (blocks, t);
+ }
+ }
+
/* Verify the GIMPLE statements in the CFG of FN. */
DEBUG_FUNCTION void
*** verify_gimple_in_cfg (struct function *f
*** 4526,4537
{
basic_block bb;
bool err = false;
! struct pointer_set_t *visited, *visited_stmts;
timevar_push (TV_TREE_STMT_VERIFY);
visited = pointer_set_create ();
visited_stmts = pointer_set_create ();
FOR_EACH_BB_FN (bb, fn)
{
gimple_stmt_iterator gsi;
--- 4583,4602
{
basic_block bb;
bool err = false;
! struct pointer_set_t *visited, *visited_stmts, *blocks;
timevar_push (TV_TREE_STMT_VERIFY);
visited = pointer_set_create ();
visited_stmts = pointer_set_create ();
+ /* Collect all BLOCKs referenced by the BLOCK tree of FN. */
+ blocks = pointer_set_create ();
+ if (DECL_INITIAL (fn-decl))
+ {
+ pointer_set_insert (blocks, DECL_INITIAL (fn-decl));
+ collect_subblocks (blocks, DECL_INITIAL (fn-decl));
+ }
+
FOR_EACH_BB_FN (bb, fn)
{
gimple_stmt_iterator gsi;
*** verify_gimple_in_cfg (struct function *f
*** 4552,4557
--- 4617,4629
err2 |= verify_gimple_phi (phi);
+ /* Only PHI arguments have locations. */
+ if (gimple_location (phi) != UNKNOWN_LOCATION)
+ {
+ error (PHI node with location);
+ err2 = true;
+ }
+
for (i = 0; i gimple_phi_num_args (phi); i++)
{
tree arg = gimple_phi_arg_def (phi, i);
*** verify_gimple_in_cfg (struct function *f
*** 4562,4567
--- 4634,4653
debug_generic_expr (addr);
err2 |= true;
}
+ location_t loc = gimple_phi_arg_location (phi, i);
+ if (virtual_operand_p (gimple_phi_result (phi))
+ loc != UNKNOWN_LOCATION)
+ {
+ error (virtual PHI with argument locations);
+ err2 = true;
+ }
+ addr = walk_tree (arg, verify_expr_location_1, blocks, NULL);
+ if (addr)
+ {
+ debug_generic_expr (addr);
+ err2 = true;
+ }
+ err2 |= verify_location (blocks, loc);
}
if (err2)
*** verify_gimple_in_cfg (struct function *f
*** 4586,4591
--- 4672,4678
}
err2 |= verify_gimple_stmt (stmt);
+ err2 |= verify_location (blocks, gimple_location (stmt));
memset (wi, 0, sizeof (wi));
wi.info = (void *) visited;
*** verify_gimple_in_cfg (struct function *f
*** 4597,4602
--- 4684,4698
err2 |= true;
}
+ memset (wi, 0, sizeof (wi));
+ wi.info = (void *) blocks;
+ addr
Re: libsanitizer mege from upstream r171973
On Thu, Jan 10, 2013 at 11:01:30AM +0400, Konstantin Serebryany wrote:
Hi,
The attached patch is the libsanitizer merge from upstream r171973.
Lots of changes. Among other things:
- slow CFI-based unwinder (off by default for now)
- more interceptors in asan (read, pread, etc)
- precise handling of memset/memcpy/etc (detects first overrun byte)
- new experimental asan allocator (off by default)
- asan now detects new/free, malloc/delete and new/delete[] mismatches
- tsan now detects more races IO operations
Patch for libsanitizer is automatically generated by libsanitizer/merge.sh
Tested with
rm -rf */{*/,}libsanitizer \
make -j 50 \
make -C gcc check-g{cc,++}
RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp'
Our internal LLVM bots (Linux, Mac and Android) are also green, but
since the changes are large something may potentially break on other
platforms.
Ok to commit?
--kcc
kcc,
Any chance that we can get http://llvm-reviews.chandlerc.com/D216 approved
and committed
into llvm so that the libsanitizer merge can also eliminate the need for the
Core Foundation
framework? It appears that you are assigned as the reviewer for that patch.
Unfortunately
diff #518 has bit-rotted against current compiler-rt svn so Alexander will need
to correct
the patch for that.
Jack
Re: [PATCH][RFC] location and BLOCK checker
On Thu, Jan 10, 2013 at 04:36:26PM +0100, Richard Biener wrote: The following is what I use to track down PR55792 - it adds verification that the GIMPLE IL only contains references to BLOCKs that are part of the functions BLOCK tree. Otherwise BLOCKs lack a reference that is seen by the garbage collector and funny things can happen. It probably should also scan DEBUG_STMTs. Do we want this in trunk or should it wait for stage1? I'd say we want it in the trunk now (well, as soon as it doesn't report regressions during bootstrap/regtest). And yes, it should check also DEBUG_STMTs, eventhough for now we completely ignore their locations. Jakub
Re: [PATCH][RFC] location and BLOCK checker
On Thu, 10 Jan 2013, Jakub Jelinek wrote:
On Thu, Jan 10, 2013 at 04:36:26PM +0100, Richard Biener wrote:
The following is what I use to track down PR55792 - it adds verification
that the GIMPLE IL only contains references to BLOCKs that are part
of the functions BLOCK tree. Otherwise BLOCKs lack a reference that is
seen by the garbage collector and funny things can happen.
It probably should also scan DEBUG_STMTs.
Do we want this in trunk or should it wait for stage1?
I'd say we want it in the trunk now (well, as soon as it doesn't report
regressions during bootstrap/regtest). And yes, it should check also
DEBUG_STMTs, eventhough for now we completely ignore their locations.
Ok, it seems it already does now.
Bootstrapped and tested on x86_64-unknown-linux-gnu with some
Fortran testsuite ICEs, LTO profiledbootstrap fails with verification
errors. Checking regular LTO bootstrap now.
Richard.
2013-01-10 Richard Biener rguent...@suse.de
* tree-cfg.c (verify_node_sharing_1): Split out from ...
(verify_node_sharing): ... here.
(verify_expr_location, verify_expr_location_1, verify_location,
collect_subblocks): New functions.
(verify_gimple_in_cfg): Verify that locations only reference
BLOCKs in the functions BLOCK tree.
Index: gcc/tree-cfg.c
===
*** gcc/tree-cfg.c.orig 2013-01-10 16:27:39.0 +0100
--- gcc/tree-cfg.c 2013-01-10 16:55:43.644221848 +0100
*** tree_node_can_be_shared (tree t)
*** 4483,4495
return false;
}
! /* Called via walk_gimple_stmt. Verify tree sharing. */
static tree
! verify_node_sharing (tree *tp, int *walk_subtrees, void *data)
{
! struct walk_stmt_info *wi = (struct walk_stmt_info *) data;
! struct pointer_set_t *visited = (struct pointer_set_t *) wi-info;
if (tree_node_can_be_shared (*tp))
{
--- 4483,4494
return false;
}
! /* Called via walk_tree. Verify tree sharing. */
static tree
! verify_node_sharing_1 (tree *tp, int *walk_subtrees, void *data)
{
! struct pointer_set_t *visited = (struct pointer_set_t *) data;
if (tree_node_can_be_shared (*tp))
{
*** verify_node_sharing (tree *tp, int *walk
*** 4503,4508
--- 4502,4516
return NULL;
}
+ /* Called via walk_gimple_stmt. Verify tree sharing. */
+
+ static tree
+ verify_node_sharing (tree *tp, int *walk_subtrees, void *data)
+ {
+ struct walk_stmt_info *wi = (struct walk_stmt_info *) data;
+ return verify_node_sharing_1 (tp, walk_subtrees, wi-info);
+ }
+
static bool eh_error_found;
static int
verify_eh_throw_stmt_node (void **slot, void *data)
*** verify_eh_throw_stmt_node (void **slot,
*** 4519,4524
--- 4527,4589
return 1;
}
+ /* Verify if the location LOCs block is in BLOCKS. */
+
+ static bool
+ verify_location (pointer_set_t *blocks, location_t loc)
+ {
+ tree block = LOCATION_BLOCK (loc);
+ if (block != NULL_TREE
+!pointer_set_contains (blocks, block))
+ {
+ error (location references block not in block tree);
+ return true;
+ }
+ return false;
+ }
+
+ /* Called via walk_tree. Verify locations of expressions. */
+
+ static tree
+ verify_expr_location_1 (tree *tp, int *walk_subtrees, void *data)
+ {
+ struct pointer_set_t *blocks = (struct pointer_set_t *) data;
+
+ if (!EXPR_P (*tp))
+ {
+ *walk_subtrees = false;
+ return NULL;
+ }
+
+ location_t loc = EXPR_LOCATION (*tp);
+ if (verify_location (blocks, loc))
+ return *tp;
+
+ return NULL;
+ }
+
+ /* Called via walk_gimple_op. Verify locations of expressions. */
+
+ static tree
+ verify_expr_location (tree *tp, int *walk_subtrees, void *data)
+ {
+ struct walk_stmt_info *wi = (struct walk_stmt_info *) data;
+ return verify_expr_location_1 (tp, walk_subtrees, wi-info);
+ }
+
+ /* Insert all subblocks of BLOCK into BLOCKS and recurse. */
+
+ static void
+ collect_subblocks (pointer_set_t *blocks, tree block)
+ {
+ tree t;
+ for (t = BLOCK_SUBBLOCKS (block); t; t = BLOCK_CHAIN (t))
+ {
+ pointer_set_insert (blocks, t);
+ collect_subblocks (blocks, t);
+ }
+ }
+
/* Verify the GIMPLE statements in the CFG of FN. */
DEBUG_FUNCTION void
*** verify_gimple_in_cfg (struct function *f
*** 4526,4537
{
basic_block bb;
bool err = false;
! struct pointer_set_t *visited, *visited_stmts;
timevar_push (TV_TREE_STMT_VERIFY);
visited = pointer_set_create ();
visited_stmts = pointer_set_create ();
FOR_EACH_BB_FN (bb, fn)
{
gimple_stmt_iterator gsi;
--- 4591,4610
{
basic_block bb;
bool err = false;
! struct pointer_set_t *visited, *visited_stmts, *blocks;
timevar_push (TV_TREE_STMT_VERIFY);
visited = pointer_set_create ();
visited_stmts = pointer_set_create ();
+
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli do...@redhat.com wrote: Also, I am not sure if this patch would be appropriate for commit, now that we are in 'regression-only' mode. But seeing this alias-template related thing not working hurts me. :) So at worst I'll schedule the patch for when stage1 opens again. It is certainly a blocker for some people here using both constexpr and template alias. (The code uses a technique documented in the upcoming 4th edition of TC++PL which is due anytime now. It would be embarrassing if GCC-4.8 didn't accept it.) -- Gaby
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli do...@redhat.com wrote:
But during the instantiation of the *members* of testint, we try to
instantiate Aliasthe_truthint, without coercing (and thus without
folding) the argument {the_truthint}. We do this using
instantiate_alias_template, called from tsubst.
The patch below makes sure instantiate_alias_template coerces the
arguments it uses to instantiate the alias template, like what I
understood you are suggesting. I have tested it without boostrap and a
full boostrap is currently running.
Hmm, is it necessary to coerce all levels as opposed to just the
innermost arguments?
-- Gaby
Re: Minor testsuite improvement
On 01/10/2013 03:31 AM, Uros Bizjak wrote: Hello! This patch tightens the expected output from the vrp dump which has the side effect of making each test's string unique. Obviously the masked failure is xfailed. OK for the trunk? Hmm, but if the SSA versions are simply i_10 then i_.*0 will still match it the same? I think you want the more elaborate 'i_\[0-9\]* . 0' here? IMO 'i_\[0-9\]+' would be better, we need at least one digit here. Yea, this is a better regexp. I'll update and verify I get the right pass/xfails. jeff
[Patch, AArch64] Set libgloss_dir for aarch64*-*-* targets
Hi,
This patch updates the top-level configuration files to explicitly set
libgloss_dir to aarch64 for aarch64*-*-* targets.
OK to commit?
Thanks,
Yufeng
2013-01-10 Yufeng Zhang yufeng.zh...@arm.com
* configure.ac: Set libgloss_dir for the aarch64*-*-* targets.
* configure: Regenerated.diff --git a/configure.ac b/configure.ac
index 02720ee..5bdf1d0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -759,6 +759,9 @@ case ${target} in
sh*-*-pe|mips*-*-pe|*arm-wince-pe)
libgloss_dir=wince
;;
+ aarch64*-*-* )
+libgloss_dir=aarch64
+;;
arm*-*-*)
libgloss_dir=arm
;;
[Ada] Remove couple of left-overs in build machinery
This removes a couple of left-overs from the pre toplevel bootstrap era. No functional changes. This paves the way for a real change that we want to enable for the 4.8.0 release. Tested on x86_64-suse-linux (w and w/o --disable-bootstrap), applied on the mainline. 2013-01-10 Eric Botcazou ebotca...@adacore.com * gcc-interface/config-lang.in (boot_language_boot_flags): Delete. * gcc-interface/Make-lang.in (BOOT_ADAFLAGS): Likewise. -- Eric BotcazouIndex: gcc-interface/config-lang.in === --- gcc-interface/config-lang.in (revision 195012) +++ gcc-interface/config-lang.in (working copy) @@ -1,5 +1,5 @@ # Top level configure fragment for GNU Ada (GNAT). -# Copyright (C) 1994-2008 Free Software Foundation, Inc. +# Copyright (C) 1994-2013 Free Software Foundation, Inc. #This file is part of GCC. @@ -21,15 +21,13 @@ # We define several parameters used by configure: # # language - name of language as it would appear in $(LANGUAGES) -# boot_language - yes if we need to build this language in stage1 +# gcc_subdir - where the gcc integration files are to be found +# boot_language - yes if we need to build this language in stage1 # compilers - value to add to $(COMPILERS) language=ada gcc_subdir=ada/gcc-interface - boot_language=yes -boot_language_boot_flags='ADAFLAGS=$(BOOT_ADAFLAGS)' - compilers=gnat1\$(exeext) gtfiles=\$(srcdir)/ada/gcc-interface/ada-tree.h \$(srcdir)/ada/gcc-interface/gigi.h \$(srcdir)/ada/gcc-interface/decl.c \$(srcdir)/ada/gcc-interface/trans.c \$(srcdir)/ada/gcc-interface/utils.c \$(srcdir)/ada/gcc-interface/misc.c Index: gcc-interface/Make-lang.in === --- gcc-interface/Make-lang.in (revision 195012) +++ gcc-interface/Make-lang.in (working copy) @@ -1,6 +1,6 @@ # Top level -*- makefile -*- fragment for GNU Ada (GNAT). # Copyright (C) 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, -# 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 +# 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 # Free Software Foundation, Inc. #This file is part of GCC. @@ -47,7 +47,6 @@ RMDIR = rm -rf # Extra flags to pass to recursive makes. COMMON_ADAFLAGS= -gnatpg -gnata WARN_ADAFLAGS= -W -Wall -BOOT_ADAFLAGS= $(COMMON_ADAFLAGS) # For native builds, the base compiler might be old and we need to arrange for # style warnings not to be flagged as errors during stage1. Cross compilers
Re: Minor testsuite improvement
On 01/10/2013 03:13 AM, Richard Biener wrote: Hmm, but if the SSA versions are simply i_10 then i_.*0 will still match it the same? I think you want the more elaborate 'i_\[0-9\]* . 0' here? Per Uros's suggestion I'm using i[0-9]+ to ensure we one or more digits. Please also open a bug for the added XFAIL. 55936. Not sure if it's a regression or not -- Diego changed the relevant code in tree-ssa-vrp around 2006. I didn't build a compiler from that era to see if VRP caught everything in one pass. jeff
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
On 01/10/2013 11:11 AM, Gabriel Dos Reis wrote: Hmm, is it necessary to coerce all levels as opposed to just the innermost arguments? No. But if you read the code, it's really only coercing the innermost level. Just the name is misleading. If this approach looks acceptable, could I replace (part of) the template argument coercing code in lookup_class_template_1 with the new coerce_template_parms_all_level I introduced in this patch? Yes. Jason
PATCH to add .gitignore
Apparently a .gitignore file was added to the gdb repository a while back, and I think it makes sense for it to be in the gcc repository as well. I made a few edits to avoid ignoring things that are actually part of the gcc repository; this isn't strictly necessary, as the ignore file only applies to untracked files, but it seems appropriate all the same. Applying to trunk. commit 5bef2461a04c1b81f63af91bc2e100e60a0b9bff Author: Jason Merrill ja...@redhat.com Date: Wed Jan 9 15:42:50 2013 -0500 * .gitignore: Import from gdb repository. diff --git a/.gitignore b/.gitignore new file mode 100644 index 000..bda55a3 --- /dev/null +++ b/.gitignore @@ -0,0 +1,42 @@ +*.diff +*.patch +*.orig +*.rej + +*~ +.#* +*# + +*.flt +*.gmo +*.info +*.la +*.lo +*.o +*.pyc +*.tmp + +.deps +.libs + +autom4te.cache +config.cache +config.h +config.intl +config.log +config.status +libtool +POTFILES +*-POTFILES + +TAGS +TAGS.sub + +.gdbinit +.gdb_history + +# ignore core files, but not java/net/protocol/core/ +core +!core/ + +lost+found diff --git a/gcc/testsuite/ada/.gitignore b/gcc/testsuite/ada/.gitignore new file mode 100644 index 000..a499072 --- /dev/null +++ b/gcc/testsuite/ada/.gitignore @@ -0,0 +1,2 @@ +# In the Ada testsuite, .a is an ada source file, not a library. +!*.a
[PATCH] Fix PR55833 + cheaper checking
The following patch fixes (?) PR55833 by recomputing irreducible loops
after every unswitch-transformation. With this testcase, when we perform
loop unswitching on the RTL level, we end up in situation where we have
a loop into which we get through loop exit of another loop which is
a part of an irreducible region. Between those two loops is something like
preheader, which isn't marked as irreducible, nor its edges.
verify_loop_structure
then ICEs because of that. Another hunk of this patch is just cheaper checking,
written by Richi.
Regtested/bootstrapped on x86_64-linux.
Zdenek, any thoughts on this?
2013-01-10 Richard Biener rguent...@suse.de
Marek Polacek pola...@redhat.com
PR rtl-optimization/55833
* loop-unswitch.c (unswitch_loops): Move loop verification...
(unswitch_single_loop): ...here. Call mark_irreducible_loops.
* gcc.dg/pr55833.c: New test.
--- gcc/loop-unswitch.c.mp 2013-01-10 16:50:28.899559875 +0100
+++ gcc/loop-unswitch.c 2013-01-10 16:50:34.203575403 +0100
@@ -145,12 +145,7 @@ unswitch_loops (void)
/* Go through inner loops (only original ones). */
FOR_EACH_LOOP (li, loop, LI_ONLY_INNERMOST)
-{
- unswitch_single_loop (loop, NULL_RTX, 0);
-#ifdef ENABLE_CHECKING
- verify_loop_structure ();
-#endif
-}
+unswitch_single_loop (loop, NULL_RTX, 0);
iv_analysis_done ();
}
@@ -370,6 +365,13 @@ unswitch_single_loop (struct loop *loop,
nloop = unswitch_loop (loop, bbs[i], copy_rtx_if_shared (cond), cinsn);
gcc_assert (nloop);
+ /* We changed the CFG. Recompute irreducible BBs and edges. */
+ mark_irreducible_loops ();
+
+#ifdef ENABLE_CHECKING
+ verify_loop_structure ();
+#endif
+
/* Invoke itself on modified loops. */
unswitch_single_loop (nloop, rconds, num + 1);
unswitch_single_loop (loop, conds, num + 1);
--- gcc/testsuite/gcc.dg/pr55833.c.mp 2013-01-10 17:23:26.016102692 +0100
+++ gcc/testsuite/gcc.dg/pr55833.c 2013-01-10 17:23:15.898073384 +0100
@@ -0,0 +1,28 @@
+/* PR rtl-optimization/55833 */
+/* { dg-do compile } */
+/* { dg-options -O3 } */
+
+int a, b, c;
+
+void foo()
+{
+unsigned d, l, *p, k = 1;
+
+if(bar())
+{
+label:
+ if((a = a = 0))
+{
+if(c)
+d = b;
+
+if (b || d ? l : k ? : 0)
+a = d = 0;
+
+goto label;
+ }
+}
+
+while(*p++)
+goto label;
+}
Marek
Re: [PATCH] Fix PR55833 + cheaper checking
On Thu, Jan 10, 2013 at 6:31 PM, Marek Polacek wrote: + /* We changed the CFG. Recompute irreducible BBs and edges. */ + mark_irreducible_loops (); This is a very expensive fix for a really unusual situation. I don't think this is the right thing to do... Ciao! Steven
PR target/55565: update failing powerpc test
I have a long diatribe on the PR as to why the current generated code is
correct, and more optimal than when the test used to pass. Bottom
line is that we now perform less branches and less loads/stores, at the
expense of one register to register move.
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55565
Jakub has been kind enough to provide an updated regex for the test.
Tested on an x86-64 Linux cross to powerpc64-linux-gnu with:
make check-gcc RUNTESTFLAGS=powerpc.exp=ppc-mov-1.c
(by first removing the { target { powerpc*... } }).
OK for trunk?
commit 692859f796d90eebd97ad97ad872bbb7b7f49c12
Author: Aldy Hernandez al...@redhat.com
Date: Thu Jan 10 11:58:37 2013 -0600
PR target/55565
* gcc.target/powerpc/ppc-mov-1.c: Update scan-assembler-not
regex.
diff --git a/gcc/testsuite/gcc.target/powerpc/ppc-mov-1.c
b/gcc/testsuite/gcc.target/powerpc/ppc-mov-1.c
index 750cf85..d9439d9 100644
--- a/gcc/testsuite/gcc.target/powerpc/ppc-mov-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/ppc-mov-1.c
@@ -1,7 +1,7 @@
/* { dg-do compile { target { powerpc*-*-* lp64 } } } */
/* { dg-options -O2 } */
-/* { dg-final { scan-assembler-not fmr \[0-9\]+,\[0-9\]+ } }
+/* { dg-final { scan-assembler-not lfd \(\[0-9\]+\),\[^\n\r\]*\[\n\r\]+\[
\t]*fmr 1,\\1\[\n\r\]+\[ \t]*blr } } */
/* Origin:Pete Steinmetz stein...@us.ibm.com */
Re: [PATCH] Fix PR55882
The problem with this intermediate inconsistency is that ... That's why only the final result should matter and need be correct. ... this can't be ensured anymore. In some cases the inconsistency between the mem attributes and what XEXP(MEM, 0) represents can't be repaired by the later bitpos adjustments, or better it can be only be repaired by falling back to the conservative side for e.g. the alignment, because we don't store enough information in the mem attributes to recover what was lost. Indeed, looking at the history of the code shows that the alignment handling via MEM_REF and get_object_alignment is an afterthought. Originally, it was set only in a few specific cases: /* We can set the alignment from the type if we are making an object, this is an INDIRECT_REF, or if TYPE_ALIGN_OK. */ and also for DECL_P and CONSTANT_CLASS_P. That was it, so this was actually the alignment of the base and disregarding BITPOS was fine. When briefly discussing this yesterday I suggested (without having the code in front of me) that it be best to simply set the mem attributes only at the very end, after having computed to final MEM address including all adjustments, instead of generating wrong mem attributes initially and hoping for the adjustments to make it come out right at the end. Maybe, but for 4.7 (and probably 4.8) I think we should be conservative and only fix the problems recently introduced. So, in the end, I think that we should go either for Richard's initial patch in this thread, possibly with a ??? comment along the lines of: /* ??? If we haven't computed the alignment yet, compute it now. Note that, if already computed above, the alignment is that of the base object of T so it is OK to have disregarded BITPOS above. However, here we are using get_object_alignment_1 which returns the precise alignment of T so we need to account for the BITPOS adjustment. */ if (!align_computed) or for Richard's initial patch + the removal of the MEM_REF block (which is midway between Richard's initial patch and Richard's last patch). -- Eric Botcazou
Re: PR target/55565: update failing powerpc test
Hi, AFAIU the regexps, they are not doing what they are supposed to do on powerpc-apple-darwin9: the assembly reads fmr f1,f0 i.e., fmr \[0-9\]+ or fmr 1 are never found. If I use fmr f?\[0-9\]+,f?\[0-9\]+, then the test fails, in line with the other powerpc. If I use lfd \(f?\[0-9\]+\),\[^\n\r\]*\[\n\r\]+\[ \t]*fmr f?1,\\1\[\n\r\]+\[ \t]*blr the test passes. Cheers, Dominique PS. IIRC some previous discussions around such darwin peculiarities the f? decoration may be too simplistic to cover all the powerpc flavors (A. Pinski may know better).
Re: Minor testsuite improvement
On 01/10/2013 03:13 AM, Richard Biener wrote:
On Thu, Jan 10, 2013 at 6:15 AM, Jeff Law l...@redhat.com wrote:
Gary Funck noted that vrp06.c has two tests with the same output. After
further investigation it was clear that expected output strings were too
lenient and were in fact masking a missed optimization.
This patch tightens the expected output from the vrp dump which has the side
effect of making each test's string unique. Obviously the masked failure is
xfailed.
OK for the trunk?
Hmm, but if the SSA versions are simply i_10 then i_.*0 will still match it
the same? I think you want the more elaborate 'i_\[0-9\]* . 0' here?
Applied using Uros's regexp. The final patch is attached for historical
purposes.
Jeff
commit 115e3f9b2615bbe7bce75e4d29638e977c10ac86
Author: law law@138bc75d-0d04-0410-961f-82ee72b054a4
Date: Thu Jan 10 19:16:33 2013 +
* gcc/dg/tree-ssa/vrp06.c: Tighten expected output. Make each
pass/fail message unique.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@195093
138bc75d-0d04-0410-961f-82ee72b054a4
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 46bae82..6df9b9c 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2013-01-10 Jeff Law l...@redhat.com
+
+ * gcc/dg/tree-ssa/vrp06.c: Tighten expected output. Make each
+ pass/fail message unique.
+
+
2013-01-10 Jason Merrill ja...@redhat.com
* ada/.gitignore: New.
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp06.c
b/gcc/testsuite/gcc.dg/tree-ssa/vrp06.c
index 1ef6c92..cb6df0d 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/vrp06.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp06.c
@@ -25,7 +25,7 @@ foo (int i, int j, int a)
return i + a + j;
}
-/* { dg-final { scan-tree-dump-times Folding predicate i_.*to 0 1 vrp1 } }
*/
-/* { dg-final { scan-tree-dump-times Folding predicate j_.*to 1 1 vrp1 } }
*/
-/* { dg-final { scan-tree-dump-times Folding predicate i_.*to 0 1 vrp1 } }
*/
+/* { dg-final { scan-tree-dump-times Folding predicate i_\[0-9\]+.*0 to 0 1
vrp1 } } */
+/* { dg-final { scan-tree-dump-times Folding predicate j_\[0-9\]+.*0 to 1 1
vrp1 } } */
+/* { dg-final { scan-tree-dump-times Folding predicate
i_\[0-9]+.*j_\[0-9\]+.* to 0 1 vrp1 { xfail *-*-* } } } */
/* { dg-final { cleanup-tree-dump vrp1 } } */
Re: [Patch, Fortran] PR 55072: [4.6/4.7/4.8 Regression] Missing internal_pack leads to wrong code with derived type
Ping! (What do we do with this little bugger?) @Paul: Was your comment 19 in the PR meant as an OK for my patch (submitted here: http://gcc.gnu.org/ml/fortran/2012-12/msg00097.html), or was it just a general agreement with the previous comments? Cheers, Janus 2012/12/16 Janus Weil ja...@gcc.gnu.org: Hi Tobias, here is a patch for a pretty bad wrong-code regression, which affects all maintained releases of gfortran. For discussion see bugzilla. 2012-12-15 Janus Weilja...@gcc.gnu.org PR fortran/55072 * trans-array.c (gfc_conv_array_parameter): No packing was done for full arrays of derived type. @@ -6995,20 +6995,14 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * this_array_result = false; /* Passing address of the array if it is not pointer or assumed-shape. */ - if (full_array_var g77 !this_array_result) + if (full_array_var g77 !this_array_result + sym-ts.type != BT_DERIVED sym-ts.type != BT_CLASS) Without experimenting more carefully, I have the gut feeling that there are still cases where we might end up with invalid code and there is missed optimization. Regarding the latter: If the variable is simply contiguous, there is no need to pack it. Hence, for type(t), allocatable :: a(:) ... call bar(a) there is no need to pack the array. Hm, ok. Do we do any packing in this case? Anyway, this sort of missed-optimization issue can be treated in a follow-up patch, I guess. (Mikael also noted a similar missed-optimization case in comment #13 of the PR.) What I'm aiming for here is primarily a patch for the wrong-code regression which is suitable for all three maintained branches. The problem with the original test case is that one has a non-CONTIGUOUS pointer: p = tgt(::2,::2) call bar(p) But that has in principle nothing to do with BT_DERIVED. Yes, the reason for the patch to handle BT_DERIVED in particular, is that the original commit which introduced the regression (i.e. r156749) messed up things for BT_DERIVED, which is what I'm reverting. In particular, I would like to see an additional test case for the first example case with ptr having the CONTIGUOUS attribute - and a check that then no packing call is invoked. I just checked this: The patched trunk seems to handle this properly and does not do any packing. However, I think there might be another problem: implicit none type t integer :: i end type t type(t), target :: tgt(4,4) type(t), pointer, contiguous :: p(:,:) p = tgt(::2,::2)! accepts invalid? end The pointer assignment of a contiguous pointer to a non-contiguous target should probably be rejected, right? Another follow-up problem ... For the second test case (comment 2, from GiBUU): Here, the problem is that full_array_var is wrongly true: call s1(OutPart(1,:)) I wonder whether some call to gfc_is_simply_contiguous could solve the problem for both issues. No, here I disagree. The problem with this one was not related to the call of s1, but of s2, where indeed a full array is passed! (For non-whole arrays one still have to ensure that one passes the correct element: call(a) should pass a-data and not a and call bar(a(:,2)) should neither pass a-data nor a but a-data + offset.) Regarding BT_CLASS: BT_CLASS - BT_TYPE (with same declared type) should already be handled via gfc_conv_subref_array_arg, which takes care of the actual type. Thus, the patched function should only be reachable for BT_CLASS - BT_CLASS. Here, packing is required for non-simply contiguous actual arguments; but after the packing, a class container has to be re-added. I think one should add a test case for this; testing declared type == actual type and declared type != actual type - and either one for both declared type being the same and for the dummy having the declared type of the ancestor of the declared type of the actual argument. And all cases for both simply contiguous arrays and (simply - or better actually) noncontiguous arrays. I'm ignoring all this for now. All I want to fix at this point is the wrong-code regression! Regarding the wrong code: I fear that some code involving non-BT_DERIVED could lead to wrong code, e.g. a(:)%x. I don't have an example for that If you find an example where stuff goes wrong (as a regression of my patch), I'll take care of it. but I fear that code which lead to the original issue (e.g. full_array_var is true although it shouldn't) is not solved via the patch. I actually don't think this is the case! Sorry for listing more my concerns that giving a proper review. Thanks for your comments, anyway. Cheers, Janus
Re: PR target/55565: update failing powerpc test
On 01/10/13 12:58, Dominique Dhumieres wrote:
Hi,
AFAIU the regexps, they are not doing what they are supposed to do
on powerpc-apple-darwin9: the assembly reads
fmr f1,f0
i.e., fmr \[0-9\]+ or fmr 1 are never found.
If I use fmr f?\[0-9\]+,f?\[0-9\]+, then the test fails,
in line with the other powerpc.
If I use lfd \(f?\[0-9\]+\),\[^\n\r\]*\[\n\r\]+\[ \t]*fmr f?1,\\1\[\n\r\]+\[
\t]*blr
the test passes.
I didn't tackle the Darwin issue since the original test was ignored on
Darwin, but I guess it doesn't hurt to handle the Darwin f register
variants.
OK for trunk?
commit 61ceeb130c2c2c342f19e716397ffddd212a0b32
Author: Aldy Hernandez al...@redhat.com
Date: Thu Jan 10 11:58:37 2013 -0600
PR target/55565
* gcc.target/powerpc/ppc-mov-1.c: Update scan-assembler-not
regex.
diff --git a/gcc/testsuite/gcc.target/powerpc/ppc-mov-1.c
b/gcc/testsuite/gcc.target/powerpc/ppc-mov-1.c
index 750cf85..f843015 100644
--- a/gcc/testsuite/gcc.target/powerpc/ppc-mov-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/ppc-mov-1.c
@@ -1,7 +1,7 @@
/* { dg-do compile { target { powerpc*-*-* lp64 } } } */
/* { dg-options -O2 } */
-/* { dg-final { scan-assembler-not fmr \[0-9\]+,\[0-9\]+ } }
+/* { dg-final { scan-assembler-not lfd \(f?\[0-9\]+\),\[^\n\r\]*\[\n\r\]+\[
\t]*fmr f?1,f?\\1\[\n\r\]+\[ \t]*blr } } */
/* Origin:Pete Steinmetz stein...@us.ibm.com */
Re: [Patch, Fortran] PR55758 - Non-C_Bool handling with BIND(C)
On Wed, Jan 9, 2013 at 12:34 AM, Tobias Burnus bur...@net-b.de wrote: Janne Blomqvist worte: On Sun, Jan 6, 2013 at 6:52 PM, Tobias Burnusbur...@net-b.de wrote: Attached is a small variation, which in addition handles the case that a non-BOOL_C LOGICAL, Bind(C) dummy argument (or result variable) is used in a procedure call. In that case, the variable is now converted to a TYPE_PRECISION == 1 variable. -- The updated patch was build and regtested successfully. Nice, this should fix a pitfall with the previous patch. I still worry about these almost-but-not-quite logicals causing weird and very hard to track down bugs. Though, it should be much less severe then with the current trunk. A slightly safer variant of the approach youdescribe above would be to convert the variable directly after the bind(c) procedure call; that should make it pretty fool-proof, AFAICS? (in some cases that would be a bit of extra useless work, but I doubt it would matter performance-wise). Well, that's not at trivial as it sounds. In particular for a Fortran-written procedure, which gets the input from C. If the variable is INTENT(IN) or if it is not modified in the procedure, it may not be touched. In order to do this, one has to implement support for a shadow variable, which has to set the real one at the end of the procedure. I don't think that this shadow-var handling is really that trivial. For actual arguments, doing the conversion back is simpler. Function results might be also a bit tricky, but that's mostly handled by the current patch, I hope. Ah, thanks for the clarification. I think if we cannot make it really bullet-proof, then it's safer to reject it outright. Do you happen to know if anyone except openmpi would be affected? If only openmpi, then we could give them a nudge and the issue would likely be fixed by the time gcc 4.8 rolls out to end users. -- Janne Blomqvist
[PATCH, i386]: Fix PR55929, unable to generate reloads for xbegin insn
Hello!
As mentioned by Andrew is PR, reload can't handle jump_insns. Expand
with %eax hard register temporary to avoid this limitation.
2012-01-10 Uros Bizjak ubiz...@gmail.com
PR target/55929
* config/i386/i386.md (xbegin): Use %eax as a temporary register.
Bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32} and
by compiling libitm/beginend.cc preprocessed source with -Os mrtm
-std=gnu++0x. Committed to mainline SVN.
Uros.
Index: config/i386/i386.md
===
--- config/i386/i386.md (revision 195091)
+++ config/i386/i386.md (working copy)
@@ -18013,19 +18013,22 @@
(define_expand xbegin
[(set (match_operand:SI 0 register_operand)
- (unspec_volatile:SI [(match_dup 1)] UNSPECV_XBEGIN))]
+ (unspec_volatile:SI [(const_int 0)] UNSPECV_XBEGIN))]
TARGET_RTM
{
rtx label = gen_label_rtx ();
- operands[1] = force_reg (SImode, constm1_rtx);
+ /* xbegin is emitted as jump_insn, so reload won't be able
+ to reload its operand. Force the value into AX hard register. */
+ rtx ax_reg = gen_rtx_REG (SImode, AX_REG);
+ emit_move_insn (ax_reg, constm1_rtx);
- emit_jump_insn (gen_xbegin_1 (operands[1], label));
+ emit_jump_insn (gen_xbegin_1 (ax_reg, label));
emit_label (label);
LABEL_NUSES (label) = 1;
- emit_move_insn (operands[0], operands[1]);
+ emit_move_insn (operands[0], ax_reg);
DONE;
})
Re: PR target/55565: update failing powerpc test
On Thu, Jan 10, 2013 at 01:40:09PM -0600, Aldy Hernandez wrote:
commit 61ceeb130c2c2c342f19e716397ffddd212a0b32
Author: Aldy Hernandez al...@redhat.com
Date: Thu Jan 10 11:58:37 2013 -0600
PR target/55565
* gcc.target/powerpc/ppc-mov-1.c: Update scan-assembler-not
regex.
Ok with:
diff --git a/gcc/testsuite/gcc.target/powerpc/ppc-mov-1.c
b/gcc/testsuite/gcc.target/powerpc/ppc-mov-1.c
index 750cf85..f843015 100644
--- a/gcc/testsuite/gcc.target/powerpc/ppc-mov-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/ppc-mov-1.c
@@ -1,7 +1,7 @@
/* { dg-do compile { target { powerpc*-*-* lp64 } } } */
/* { dg-options -O2 } */
-/* { dg-final { scan-assembler-not fmr \[0-9\]+,\[0-9\]+ } }
+/* { dg-final { scan-assembler-not lfd \(f?\[0-9\]+\),\[^\n\r\]*\[\n\r\]+\[
\t]*fmr f?1,f?\\1\[\n\r\]+\[ \t]*blr } } */
The ,f?
before \\1 removed. The optional f will be matched already from \\1.
Jakub
Re: PR target/55565: update failing powerpc test
PS. IIRC some previous discussions around such darwin peculiarities the f? decoration may be too simplistic to cover all the powerpc flavors (A. Pinski may know better). I have found the links for that: r168960 (pr41146). A. Pinski asked to add %?. I don't know which ppc platform uses it and if it does also for floating-point register, nevertheless the following regexp lfd \(%?f?\[0-9\]+\),\[^\n\r\]*\[\n\r\]+\[ \t]*fmr %?f?1,\\1\[\n\r\]+\[ \t]*blr also works for darwin. TIA Dominique
Re: PR target/55565: update failing powerpc test
On Thu, Jan 10, 2013 at 11:57 AM, Dominique Dhumieres domi...@lps.ens.fr wrote: PS. IIRC some previous discussions around such darwin peculiarities the f? decoration may be too simplistic to cover all the powerpc flavors (A. Pinski may know better). I have found the links for that: r168960 (pr41146). A. Pinski asked to add %?. I don't know which ppc platform uses it and if it does also for floating-point register, nevertheless the following regexp % is only used when -mregnames is used and then f is also included. Thanks, Andrew lfd \(%?f?\[0-9\]+\),\[^\n\r\]*\[\n\r\]+\[ \t]*fmr %?f?1,\\1\[\n\r\]+\[ \t]*blr also works for darwin. TIA Dominique
patch to fix PR55672
The following patch fixes
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55672
The patch was successfully tested and bootstrapped on x86-64 (with and
without -fstack-check=generic).
Committed as rev. 195095.
2013-01-10 Vladimir Makarov vmaka...@redhat.com
PR rtl-optimization/pr55672
* lra-eliminations.c (mark_not_eliminable): Permit addition with
const to be eliminable.
2013-01-10 Vladimir Makarov vmaka...@redhat.com
PR rtl-optimization/pr55672
* gcc.target/i386/pr55672.c: New.
Index: lra-eliminations.c
===
--- lra-eliminations.c (revision 195058)
+++ lra-eliminations.c (working copy)
@@ -657,7 +657,7 @@ mark_not_eliminable (rtx x)
case PRE_MODIFY:
if (REG_P (XEXP (x, 0)) REGNO (XEXP (x, 0)) FIRST_PSEUDO_REGISTER)
/* If we modify the source of an elimination rule, disable
- it. Do the same if it is the source and not the hard frame
+ it. Do the same if it is the source and not the hard frame
register. */
for (ep = reg_eliminate;
ep reg_eliminate[NUM_ELIMINABLE_REGS];
@@ -716,7 +716,10 @@ mark_not_eliminable (rtx x)
ep reg_eliminate[NUM_ELIMINABLE_REGS];
ep++)
if (ep-to_rtx == SET_DEST (x)
- SET_DEST (x) != hard_frame_pointer_rtx)
+ SET_DEST (x) != hard_frame_pointer_rtx
+ (GET_CODE (SET_SRC (x)) != PLUS
+ || XEXP (SET_SRC (x), 0) != SET_DEST (x)
+ || ! CONST_INT_P (XEXP (SET_SRC (x), 1
setup_can_eliminate (ep, false);
}
Index: testsuite/gcc.target/i386/pr55672.c
===
--- testsuite/gcc.target/i386/pr55672.c (revision 0)
+++ testsuite/gcc.target/i386/pr55672.c (working copy)
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options -O -fstack-check=generic } */
+
+int main ()
+{
+ int x[8];
+ if (x[0] != 4)
+__builtin_abort ();
+ return 0;
+}
Re: [v3] Fix management of non empty hash functor
Hi
Here is an other version of this patch. Indeed there were no need
to expose many stuff public. Inheriting from _Hash_code_base is fine, it
is not final and it deals with EBO itself. I only kept usage of
_Hashtable_ebo_helper when embedding H2 functor. As it is an extension
we could have impose it not to be final but it doesn't cost a lot to
deal with it. Finally I only needed a single friend declaration to get
access to the H2 part of _Hash_code_base.
I didn't touch the default cache policy for the moment except
reducing constraints on the hash functor. I prefer to submit an other
patch to change when we cache or not depending on the hash functor
expected performance.
I also took the time to replace some typedef expressions with using
ones. I really know what is the rule about using one or the other but I
remembered that Benjamin spent quite some time changing typedef in using
so I prefer to stick to this approach in this file, even if there are
still some typedef left.
Tested under linux x86_64 normal and debug modes.
2013-01-10 François Dumont fdum...@gcc.gnu.org
* include/bits/hashtable_policy.h (_Local_iterator_base): Use
_Hashtable_ebo_helper to embed necessary functors into the
local_iterator when necessary. Pass information about functors
involved in hash code by copy.
* include/bits/hashtable.h (__cache_default): Do not cache for
builtin integral types unless the hash functor is not noexcept
qualified or is not default constructible. Adapt static assertions
and local iteraror instantiations.
* include/debug/unordered_set
(std::__debug::unordered_set::erase): Detect local iterators to
invalidate using contained node rather than generating a dummy
local_iterator instance.
(std::__debug::unordered_multiset::erase): Likewise.
* include/debug/unordered_map
(std::__debug::unordered_map::erase): Likewise.
(std::__debug::unordered_multimap::erase): Likewise.
* testsuite/performance/23_containers/insert_erase/41975.cc: Test
std::tr1 and std versions of unordered_set regardless of any
macro. Add test on default cache behavior.
* testsuite/performance/23_containers/insert/54075.cc: Likewise.
* testsuite/23_containers/unordered_set/instantiation_neg.cc:
Adapt line number.
* testsuite/23_containers/unordered_set/
not_default_constructible_hash_neg.cc: New.
* testsuite/23_containers/unordered_set/buckets/swap.cc: New.
If you agree with the patch tell me where and when to apply it.
François
On 01/04/2013 12:17 PM, Paolo Carlini wrote:
Hi,
On 12/13/2012 10:32 PM, François Dumont wrote:
Hi
As part of a performance patch proposed in an other mailing
thread was a patch to improve management of hash functor with state.
This part is I think less sensible than the performance patch so I
propose it independently. I only would like to commit the
modification on the performance tests here if you don't mind.
Thanks to this patch caching the hash code or not doesn't depend
on the hash functor to be empty of final anymore. I only keep the
default constructible condition so that local_iterator can be default
constructible, considering it is a Standard request.
I'm finally having a closer look at this work of yours (sorry aboutt
the delay!) and I think we want something similar for 4.8.0. However,
to be honest, I'm not convinced we are implementing the general idea
in the best way, in particular I don't like the much more complex
access control structure, _Hash_code_base loses encapsulation, etc.
Did you consider maybe adding friend declarations in a few places?
Jon, do you have suggestiong? The idea of managing to get rid of the
empty !final requirement for dispatching seems right to me.
By the way, I'm also not convinced that is_integral is the right
category, I think is_scalar for example is better: pointers are common
and very similar in terms of std::hash, likewise floating point
quantities (with the possible exception of long double, but I don't
think we should spend time on it).
Paolo.
Index: include/bits/hashtable_policy.h
===
--- include/bits/hashtable_policy.h (revision 195097)
+++ include/bits/hashtable_policy.h (working copy)
@@ -1,6 +1,6 @@
// Internal policy header for unordered_set and unordered_map -*- C++ -*-
-// Copyright (C) 2010, 2011, 2012 Free Software Foundation, Inc.
+// Copyright (C) 2010-2013 Free Software Foundation, Inc.
//
// This file is part of the GNU ISO C++ Library. This library is free
// software; you can redistribute it and/or modify it under the
@@ -202,7 +202,7 @@
templatetypename _Value, bool _Cache_hash_code
struct _Node_iterator_base
{
- typedef _Hash_node_Value, _Cache_hash_code __node_type;
+ using __node_type = _Hash_node_Value, _Cache_hash_code;
__node_type* _M_cur;
@@ -282,7 +282,7 @@
Re: [PATCH] Fix PR55833 + cheaper checking
Hi, On Thu, Jan 10, 2013 at 6:31 PM, Marek Polacek wrote: + /* We changed the CFG. Recompute irreducible BBs and edges. */ + mark_irreducible_loops (); This is a very expensive fix for a really unusual situation. I don't think this is the right thing to do... I agree -- at the very least, unswitch_single_loop should check whether there is any possiblity it could have affected irreducible loops information (this should only be the case when some already existing irreducible loop is altered in the progress). Which is what it (or more precisely, remove_path function used by it) tries to do -- so is should be sufficient to check why this fails for the considered testcase, and make sure the situation is correctly detected, Zdenek
Re: Support for MIPS r5900
Hello Jeff, If you're using something from the Cygnus port, then it would be covered by the blanket copyright assignment Cygnus had in place with the FSF. If there are any doubts about the origin of a hunk of GCC code I could probably pull out the old sources to determine if it came from Cygnus's code base or not. Can you please tell me whether the following lines of the patches are covered by the blanket copyright assignment? Lines 335-533 of GCC patch (mips_r5900_lengthen_loops()): http://pastie.org/5664783 Lines 410-565 and 581-589 of binutils patch (check_short_loop()): http://pastie.org/5664824 The patches are from the second DVD of Sony's Linux Toolkit for the PS2. Best regards Jürgen
Re: [RFC PATCH] Implementing ifunc target hook
On 10/01/2013, at 12:24 AM, Alexander Ivchenko wrote:
Config/linux-android.h is a better place for this declaration.
I was wrong here. Config/linux-android.h is not a re-includable header, and
is not fit for function declarations.
That wouldn't help, I got the following error:
In file included from ../../.././gcc/tm.h:24:0,
from [..]/src/gcc/libgcc/generic-morestack-thread.c:29:
[..]/src/gcc/libgcc/../gcc/config/linux-android.h:62:1: error: unknown
type name ‘bool’
extern bool linux_android_has_ifunc_p (void);
^
Anyway, linux-protos.h looks to me as a good thing to have (e.g. for
libc_has_function hook, that is
supposed to be commited in a near future) for declaring linux (and
Android) specific versions of hooks..
OK, I agree. In theory linux_android_has_ifunc_p could've been placed into
linux-android-protos.h, but having a separate file (from linux-protos.h) just
for 1-2 declarations is not justified.
+ It returns true if the target supports GNU indirect functions.\n\
+The support includes the assembler, linker and dynamic linker.\n\
+The default value of this hook is defined as for the host machine.,
Are your sure the last sentence is correct? It seems the default value for
this hook is based on which libc is being used. Maybe it would be more
accurate to say The default value of this hook is based on target's libc.?
Well yes, you are right that the default value depends on version of
libc, but this version
is checked on the configure time for the host machine
(HAVE_GNU_INDIRECT_FUNCTION),
No. And even if that was the case that would be a bug.
HAVE_GNU_INDIRECT_FUNCTION is set based on default_gnu_indirect_function
variable defined in config.gcc (unless overridden with a configure argument).
This variable is set to true for targets that support IFUNCs irrespective of
host and host's libc.
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -637,6 +637,11 @@ case ${target} in
native_system_header_dir=/include
;;
esac
+ case $target in
+*linux*)
+ tm_p_file=${tm_p_file} linux-protos.h
+ ;;
+ esac
# glibc / uclibc / bionic switch.
# uclibc and bionic aren't usable for GNU/Hurd and neither for GNU/k*BSD.
case $target in
@@ -662,8 +667,10 @@ case ${target} in
# Add Android userspace support to Linux targets.
case $target in
*linux*)
+ tmake_file=${tmake_file} t-linux-android
tm_file=$tm_file linux-android.h
extra_options=$extra_options linux-android.opt
+ extra_objs=$extra_objs linux-android.o
;;
esac
# Enable compilation for Android by default for *android* targets.
Also scan config.gcc for any other occurences of linux-android.h and add same
changes there. I believe only arm*-*-linux-* is affected.
--- a/gcc/config/host-linux.c
+++ b/gcc/config/host-linux.c
@@ -23,7 +23,6 @@
#include hosthooks.h
#include hosthooks-def.h
-
/* Linux has a feature called exec-shield-randomize that perturbs the
address of non-fixed mapped segments by a (relatively) small amount.
The feature is intended to make it harder to attack the system with
Don't change formatting unless you are editing the code around it.
diff --git a/gcc/config/linux-android.c b/gcc/config/linux-android.c
new file mode 100644
index 000..47cc50a
--- /dev/null
+++ b/gcc/config/linux-android.c
...
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3. If not see
+http://www.gnu.org/licenses/. */
+
+#include config.h
+#include system.h
+#include coretypes.h
+#include tm.h
Add '#include linux-protos.h'. Remember to update t-linux-android.
+
+/* Android does not support GNU indirect functions. */
+
+bool
+linux_android_has_ifunc_p (void)
+{
+ return TARGET_ANDROID ? false : HAVE_GNU_INDIRECT_FUNCTION;
+}
Please send one last version for the final review, primarily to double check
your changes to arm*-linux-* to avoid breaking it.
Thanks!
--
Maxim Kuvyrkov
Re: Support for MIPS r5900
Jürgen, Adding the binutils list as more appropriate for some concerns discussed here. ll, sc, dmult, ddiv, cvt.w.s, 64 bit FPU instructions. ll and sc is disabled with -mno-llsc and works. cvt.w.s is replaced by trunc.w.s. This seems to work. Probably showing my ignorance, but I couldn't see this in the patch. This has raised my attention -- AFAICS the binutils change recently approved correctly disables DMULT, DDIV, CVT.W.S, etc. for -march=r5900, but does not do that for LL or SC. I think that should be fixed. And I gather LLD and SCD should then be disabled as well. The glibc can only be compiled with support for ll and sc. The Linux kernel successfully emulates these instructions. When compiling GCC for mips*r5900*-elf (i.e. not Linux), the instructions ll/sc and lld/scd are disabled by my patch. That a particular OS emulates some instructions in software does not necessarily make them a part of the architecture. GAS needs to support any target environment, including bare iron, and as such should closely match the hardware implementation. I think the right place to address it is glibc. The library can be built for the base MIPS I ISA that did not have LL or SC instructions either and therefore already has some provisions in place to override the processor/ISA selection for the code fragments in question so that the instructions otherwise missing from the target hardware selected are nevertheless assembled successfully. This is currently enabled for the o32 ABI, where .set mips2 is used to enable the assembly of LL and SC. Now if that failed for you, then it's a plain bug in GAS that should be fixed. Can you therefore check whether a piece like: .setmips2 ll $2, ($3) assembles correctly with -march=r5900? Please note that the issue of LLD and SCD remains open -- these instructions are a part of the base MIPS III 64-bit ISA and therefore they are assumed by glibc and elsewhere to be present, and they are not emulated by Linux. So not only you'll have to fix up glibc to surround their use with .set mips3 for the n64 and n32 ABIs (please note that .set mips3 is needed for LL and SC for these ABIs as well to avoid a miscalculation of addresses where applicable), but you'll have to add emulation code to Linux as well. And in any case I insist that the instructions are correctly marked in the opcode table. Things would get more complicated if one wanted to run a real OS such as Linux on the R5900 and let the kernel FP emulator handle the missing double FP automagically -- this is a little bit out of scope here as regular -mdouble-float would then just do, but makes me wonder whether -mfp32 should really be enforced (as opposed to just defaulted) for the R5900, hmm... I tried to emulate the 64 Bit FPU when the real 32 Bit FPU was enabled in Linux. There are 2 problems with this: 1. When the program starts, I don't know if it needs a 64 Bit or 32 Bit FPRs. So registers are initialized for 32 bit. When dmfc0 or dmtc0 appears, I need to emulate them using 32 Bit FPU, because some 32 bit programs use these instructions with a 32 Bit FPU (e.g. Linux 2.6.35 kernel and Debian 5.0). When a 64 bit calculation instructions appears, I need to switch from 32 bit FPRs to 64 bit FPRs. When the program used 32 bit instructions with the odd FPRs, there is no way to reconstruct the overwritten part of the 64 bit FPRs. The mode of the FPU is determined by the ABI -- o32 programs use the 32-bit configuration (CP0.Status.FR set to 0) and n64/n32 programs use the 64-bit arrangement (CP0.Status.FR set to 1). That's already handled correctly by the kernel, by configuring the FPU on a process-by-process basis according to data obtained from the ELF file header of the executable run. Of course all double arithmetic would have to be handled by the emulator, by trapping the Unimplemented Operation exception. This would clearly be a new mode of operation and not supported out of the box with current code as that would have to be tweaked to handle the case where only half the register state is stored in hardware. 2. Some undefined instructions (e.g. c.eq.d) doesn't lead to an exception on an r5900, but have undefined behavior. So there is no emulation possible. It just calculates random stuff. Oh well, that rules out any practical use of the FPU under Linux then. So the FPU needs to be disabled and completely emulated by the kernel, because then all FPU instructions lead to an exception. This is working with Linux 2.6 on PS2. Naturally, as long as they got the Coprocessor Unusable exception right. There are even more problems when running unchanged code from official Fedora 12 on PS2, because of some different opcode encoding. The users of my PS2 Linux 2.6 complain about low speed, because many instructions are emulated. I need some fast
Mention of gcc 4.5 as first version with plugin support
Currently, the plugin documentation doesn't mention that it requires gcc 4.5 or later. The patch to add this statement is attached and here's the ChangeLog: 2013-01-10 Dave Johansen davejohan...@gmail.com * gcc/doc/plugins.texi: Added gcc.4.5.0 or later note to Plugins documentation. plugins_doc.patch Description: Binary data
Re: Support for MIPS r5900
On 01/10/2013 03:58 PM, Jürgen Urban wrote: Hello Jeff, If you're using something from the Cygnus port, then it would be covered by the blanket copyright assignment Cygnus had in place with the FSF. If there are any doubts about the origin of a hunk of GCC code I could probably pull out the old sources to determine if it came from Cygnus's code base or not. Can you please tell me whether the following lines of the patches are covered by the blanket copyright assignment? Lines 335-533 of GCC patch (mips_r5900_lengthen_loops()): http://pastie.org/5664783 Lines 410-565 and 581-589 of binutils patch (check_short_loop()): http://pastie.org/5664824 The patches are from the second DVD of Sony's Linux Toolkit for the PS2. Neither of those would be covered by the blanket assignment as to the best of my knowledge they were not written by a Cygnus/Red Hat engineer while working for Cygnus/Red Hat. Clearly they're working around a chip bug, which seems to be documented reasonably well in a comment. Given that documentation you could write your own check for that processor bug. jeff
