Re: [PATCH] Pass correct memory attributes for build constant
test.c:
extern bar(unsigned char p[3][2]);
void foo(int i)
{
unsigned char data[3][2] = {{1,1}, {1,0}, {1,1}};
bar(data);
}
First, note, I'm not an ARM expert. However, the first question I
have is are we sure the initializer is always going to be suitably
aligned? What guarantees this initializer is going to have 32 bit
It's a ARRAY_TYPE for the data and ARM require 32-bit align for that.
(Aligned by DATA_ALIGNMENT as Jan say.)
I think that needs to be settled first, then we need to verify that
the trees are correctly carrying that alignment requirement around
and that the code uses it appropriately (and I have my doubts
because EXP is a CONSTRUCTOR element and those seem to be largely
ignored in the code we're looking to change).
The key problem is `exp` don't have right alignment info, but `decl` have,
we can observe this in the code:
varasm.c
3166 /* Construct the VAR_DECL associated with the constant. */
3167 decl = build_decl (UNKNOWN_LOCATION, VAR_DECL, get_identifier (label),
3168 TREE_TYPE (exp));
3169 DECL_ARTIFICIAL (decl) = 1;
3170 DECL_IGNORED_P (decl) = 1;
3171 TREE_READONLY (decl) = 1;
3172 TREE_STATIC (decl) = 1;
3173 TREE_ADDRESSABLE (decl) = 1;
...
3181 if (TREE_CODE (exp) == STRING_CST)
3182 {
3183 #ifdef CONSTANT_ALIGNMENT
3184 DECL_ALIGN (decl) = CONSTANT_ALIGNMENT (exp, DECL_ALIGN (decl));
3185 #endif
3186 }
3187 else
3188 align_variable (decl, 0);
`decl` get alignment info here but `exp` doesn't.
...
3203 rtl = gen_const_mem (TYPE_MODE (TREE_TYPE (exp)), symbol);
3204 set_mem_attributes (rtl, exp, 1);
but here, we use `exp` to set memory attribute for MEM rtl.
I would also strongly recommend turning your testcase into something
we can add to the testsuite.
If you look in gcc/testsuite/gcc.target/arm you'll see several
examples. I think you just want to compile this down to assembly
code with the optimizer enabled, then verify there is no call to
memcpy in the resulting output. 20030909-1.c would seem to be a
reasonable example of a test that does something similar.
Hmmm, it's not target dependent problem, but this problem only can
observe by some target since not every target use MEM_ALIGN info for code gen,
the most common case is movmem pattern, they use alignment info to
decide expand or not.
So I am not sure is does it reasonable to make a testcase for target?
[tree-optimization/61607] Look through SSA_NAME_VALUE chains
SSA_NAME_VALUE is, in effect, a chain of values. ie, it's possible for
SSA_NAME_VALUE of any given SSA_NAME to refer to another SSA_NAME. In
many cases it is advantageous to look deeper into those chains,
particularly when simplifying conditionals for jump threading.
The problem with simply following the chains, is they can have loops.
This can occur when we're threading across a loop backedge.
I did some fairly simple experiments which showed that chains of 0, or 1
element are by far the most common. chains of 2 elements are rare, but
do occur in practice (such as pr61607). Chains of 2 elements
consistently have loops.
So this patch just looks up to two elements deep in the chain and avoids
the complication of explicitly looking for loops. This allows us to
pick up the obvious equivalency in pr61607 and gets the jumps outside
the loop fully threaded.
Bootstrapped and regression tested on x86_64-unknown-linux-gnu.
Installed on the trunk.
commit 3e2754da946eb64d7a1d30548c9b6119cda3a014
Author: Jeff Law l...@redhat.com
Date: Sun Jun 29 23:35:50 2014 -0600
tree-optimization/61607
* tree-ssa-threadedge.c (simplify_control_stmt_condition): Look
deeper into the SSA_NAME_VALUE chain.
tree-optimization/61607
* gcc.dg/tree-ssa/pr61607.c: New test.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 4cc167a..6dfe1d3 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2014-06-30 Jeff Law l...@redhat.com
+
+ tree-optimization/61607
+ * tree-ssa-threadedge.c (simplify_control_stmt_condition): Look
+ deeper into the SSA_NAME_VALUE chain.
+
2014-06-30 Zhenqiang Chen zhenqiang.c...@linaro.org
* loop-invariant.c (get_inv_cost): Handle register class.
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 5a9d73a..1df9d4e 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-06-30 Jeff Law l...@redhat.com
+
+ tree-optimization/61607
+ * gcc.dg/tree-ssa/pr61607.c: New test.
+
2014-06-30 Zhenqiang Chen zhenqiang.c...@linaro.org
* ira-loop-pressure.c: New test.
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr61607.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr61607.c
new file mode 100644
index 000..ec00f51
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr61607.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options -Os -fno-tree-fre -fdump-tree-dom1 } */
+
+void foo(int *);
+void f2(int dst[3], int R)
+{
+ int i, inter[2];
+ _Bool inter0p = 0;
+ _Bool inter1p = 0;
+ for (i = 1; i R; i++)
+{
+ inter0p = 1;
+ inter1p = 1;
+}
+ if (inter0p)
+inter[0] = 1;
+ if (inter1p)
+inter[1] = 1;
+ foo(inter);
+}
+
+
+/* There should be precisely two conditionals. One for the loop condition
+ and one for the test after the loop. Previously we failed to eliminate
+ the second conditional after the loop. */
+/* { dg-final { scan-tree-dump-times if 2 dom1} } */
+
+/* { dg-final { cleanup-tree-dump dom1 } } */
+
diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c
index a76a7ce..9807b42 100644
--- a/gcc/tree-ssa-threadedge.c
+++ b/gcc/tree-ssa-threadedge.c
@@ -542,16 +542,26 @@ simplify_control_stmt_condition (edge e,
/* Get the current value of both operands. */
if (TREE_CODE (op0) == SSA_NAME)
{
- tree tmp = SSA_NAME_VALUE (op0);
- if (tmp)
- op0 = tmp;
+ for (int i = 0; i 2; i++)
+ {
+ if (TREE_CODE (op0) == SSA_NAME
+ SSA_NAME_VALUE (op0))
+ op0 = SSA_NAME_VALUE (op0);
+ else
+ break;
+ }
}
if (TREE_CODE (op1) == SSA_NAME)
{
- tree tmp = SSA_NAME_VALUE (op1);
- if (tmp)
- op1 = tmp;
+ for (int i = 0; i 2; i++)
+ {
+ if (TREE_CODE (op1) == SSA_NAME
+ SSA_NAME_VALUE (op1))
+ op1 = SSA_NAME_VALUE (op1);
+ else
+ break;
+ }
}
if (handle_dominating_asserts)
@@ -625,10 +635,17 @@ simplify_control_stmt_condition (edge e,
It is possible to get loops in the SSA_NAME_VALUE chains
(consider threading the backedge of a loop where we have
a loop invariant SSA_NAME used in the condition. */
- if (cached_lhs
- TREE_CODE (cached_lhs) == SSA_NAME
- SSA_NAME_VALUE (cached_lhs))
- cached_lhs = SSA_NAME_VALUE (cached_lhs);
+ if (cached_lhs)
+ {
+ for (int i = 0; i 2; i++)
+ {
+ if (TREE_CODE (cached_lhs) == SSA_NAME
+ SSA_NAME_VALUE (cached_lhs))
+ cached_lhs = SSA_NAME_VALUE (cached_lhs);
+ else
+ break;
+ }
+ }
/* If we're dominated by a suitable ASSERT_EXPR, then
update CACHED_LHS appropriately. */
[Committed][AArch64] Fix register clobber in, aarch64_ashr_sisd_or_int_mode3 split.
Hi,
Fixing PR target/61633
The two split patterns associated with aarch64_ashr_sisd_or_int_mode3
split the instruction into a NEG followed by an SHL. The split uses one
of the input operands as a scratch register to hold the output of the
NEG resulting in register corruption.
This patch adjusts the splits to use the output operand as the scratch
register.
Regressed aarch64-none-elf. Committed to trunk as r212137.
Back port to 4.9 coming shortly.
/Marcus
2014-06-30 Marcus Shawcroft marcus.shawcr...@arm.com
PR target/61633
* config/aarch64/aarch64.md (*aarch64_ashr_sisd_or_int_mode3):
Add alternative; make early clobber. Adjust both split patterns
to use operand 0 as the working register.diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 8705ee9..3eb783c 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3057,17 +3057,18 @@
;; Arithmetic right shift using SISD or Integer instruction
(define_insn *aarch64_ashr_sisd_or_int_mode3
- [(set (match_operand:GPI 0 register_operand =w,w,r)
+ [(set (match_operand:GPI 0 register_operand =w,w,w,r)
(ashiftrt:GPI
- (match_operand:GPI 1 register_operand w,w,r)
- (match_operand:QI 2 aarch64_reg_or_shift_imm_di Uscmode,w,rUscmode)))]
+ (match_operand:GPI 1 register_operand w,w,w,r)
+ (match_operand:QI 2 aarch64_reg_or_shift_imm_di Uscmode,w,0,rUscmode)))]
@
sshr\t%rtn0vas, %rtn1vas, %2
#
+ #
asr\t%w0, %w1, %w2
- [(set_attr simd yes,yes,no)
- (set_attr type neon_shift_immq,neon_shift_regq,shift_reg)]
+ [(set_attr simd yes,yes,yes,no)
+ (set_attr type neon_shift_immq,neon_shift_regq,neon_shift_regq,shift_reg)]
)
(define_split
@@ -3076,11 +3077,13 @@
(match_operand:DI 1 aarch64_simd_register)
(match_operand:QI 2 aarch64_simd_register)))]
TARGET_SIMD reload_completed
- [(set (match_dup 2)
+ [(set (match_dup 3)
(unspec:QI [(match_dup 2)] UNSPEC_SISD_NEG))
(set (match_dup 0)
-(unspec:DI [(match_dup 1) (match_dup 2)] UNSPEC_SISD_SSHL))]
-
+(unspec:DI [(match_dup 1) (match_dup 3)] UNSPEC_SISD_SSHL))]
+{
+ operands[3] = gen_lowpart (QImode, operands[0]);
+}
)
(define_split
@@ -3089,11 +3092,13 @@
(match_operand:SI 1 aarch64_simd_register)
(match_operand:QI 2 aarch64_simd_register)))]
TARGET_SIMD reload_completed
- [(set (match_dup 2)
+ [(set (match_dup 3)
(unspec:QI [(match_dup 2)] UNSPEC_SISD_NEG))
(set (match_dup 0)
-(unspec:SI [(match_dup 1) (match_dup 2)] UNSPEC_SSHL_2S))]
-
+(unspec:SI [(match_dup 1) (match_dup 3)] UNSPEC_SSHL_2S))]
+{
+ operands[3] = gen_lowpart (QImode, operands[0]);
+}
)
(define_insn *aarch64_sisd_ushl
--
1.7.9.5
Re: [Patch ARM/testsuite 03/22] Add binary operators: vadd, vand, vbic, veor, vorn, vorr, vsub.
I'd rather drop the scan-assembler. I'm not convinced that the fragile
nature of this is required. Can you add a note to the README that says
that this is meant to be a complete execution test for the Advanced
SIMD intrinsics and does not cover all the assembler that is
Sure.
generated. If we have issues and regressions, we add specific directed
tests rather than carrying more noise as you've just mentioned. Any
thoughts ?
I'm not sure if it's going to be really fragile. But for sure it will
be difficult to read if it happens that we have to conditionalize the
scan-asm depending on the optim level (e.g. if the compiler prefers to
use core registers at some optimization levels).
For e.g. at O0 we don't generate vorn , vbic anymore because combine
doesn't kick in at O0.
For cases like vadd_{s/u}64 where there is scope of overlap with the
core registers then yes, surely there is a chance that this will be
fragile. So we probably need to change the structure for those set of
tests which may not be easy.
OTOH, adding such tests systematically now is probably easier than
waiting for a bug report and then adding a new test that wouldn't
cover all the variants.
True, but I don't like the noise with scan-assembler where it doesn't
work for good reasons :( And I'd rather not just add it in brute
force. An alternative suggestion I received was something like
-fno-vect-cost-model that appears to ignore the cost model thereby
giving deterministic test results. Unfortunately doing something like
that would be a bit painful in the backend(s).
Maybe we can drop the scan-asm directives for now, add all the
existing tests, and as a 2nd pass add scan-asm directives. It could be
easier to review and introduce less PRs at once :-)
Adding scan-asm on a case by case basis would probably be ok -
(especially for some of the more esoteric ones like sqdmlal where you
want the multiply accumulate to be generated) .
So if you are happy with the whole series, I could:
+ Move the tests to gcc.target/arm/ to gcc.target/aarch64 if the
AArch64 maintainers agree. For the extra AArch64 variants guard them
with #ifdef __aarch64__ #endif.
- update the README so say it's execution-only at least for the time being
- remove scan-asm from vadd.c
- commit the series I've posted so far
I'd like to finish reviewing the whole lot.
- continue the conversion
- push hopefully commit all the tests I have so far
- have a 2nd pass on all the tests and add scan-asm directives,
checking what kind of problems it raises
That makes more sense , then we know what problems we have rather than
letting this whole series get stuck waiting for everything to come
together.
+ Add a README in gcc.target/arm stating that the Advanced SIMD
intrinsics tests are in gcc.target/aarch64.
- possibly add tests for ACLE (would be in another directory)
See gcc.target/arm/acle. Maybe add these tests there.
- add missing aarch32 intrinsics if any
- add missing aarch64 intrinsics
Yes, that sounds like a plan with some minor changes as suggested
above in lines beginning with a +, the absence of a + implies a
comment.
Thanks for working on this and pushing this forward.
regards
Ramana
What do you think?
Thanks,
Christophe.
diff --git a/gcc/testsuite/gcc.target/arm/neon-intrinsics/vand.c
b/gcc/testsuite/gcc.target/arm/neon-intrinsics/vand.c
new file mode 100644
index 000..e7e65dd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/neon-intrinsics/vand.c
@@ -0,0 +1,45 @@
+#define INSN_NAME vand
+#define TEST_MSG VAND/VANDQ
+
+#include binary_op.inc
+
+/* Expected results. */
+VECT_VAR_DECL(expected,int,8,8) [] = { 0x0, 0x0, 0x2, 0x2,
+ 0x0, 0x0, 0x2, 0x2 };
+VECT_VAR_DECL(expected,int,16,4) [] = { 0xfff0, 0xfff0, 0xfff0, 0xfff0 };
+VECT_VAR_DECL(expected,int,32,2) [] = { 0x0, 0x1 };
+VECT_VAR_DECL(expected,int,64,1) [] = { 0x60 };
+VECT_VAR_DECL(expected,uint,8,8) [] = { 0x10, 0x10, 0x10, 0x10,
+ 0x14, 0x14, 0x14, 0x14 };
+VECT_VAR_DECL(expected,uint,16,4) [] = { 0x10, 0x10, 0x12, 0x12 };
+VECT_VAR_DECL(expected,uint,32,2) [] = { 0x20, 0x20 };
+VECT_VAR_DECL(expected,uint,64,1) [] = { 0x0 };
+VECT_VAR_DECL(expected,poly,8,8) [] = { 0x33, 0x33, 0x33, 0x33,
+ 0x33, 0x33, 0x33, 0x33 };
+VECT_VAR_DECL(expected,poly,16,4) [] = { 0x, 0x, 0x, 0x };
+VECT_VAR_DECL(expected,hfloat,32,2) [] = { 0x, 0x };
+VECT_VAR_DECL(expected,int,8,16) [] = { 0xf0, 0xf0, 0xf2, 0xf2,
+ 0xf4, 0xf4, 0xf6, 0xf6,
+ 0xf0, 0xf0, 0xf2, 0xf2,
+ 0xf4, 0xf4, 0xf6, 0xf6 };
+VECT_VAR_DECL(expected,int,16,8) [] = { 0xffe0, 0xffe0, 0xffe0, 0xffe0,
+ 0xffe4, 0xffe4, 0xffe4, 0xffe4 };
+VECT_VAR_DECL(expected,int,32,4) [] = {
[DOC Patch] Explicit Register Variables
I don't have permissions to commit this patch, but I do have a release
on file with the FSF.
Problem description:
The text for using Explicit Register Variables is confusing, redundant,
and fails to make certain essential information clear.
Some specific problems:
- There is no reason to call this topic Explicit Reg Vars instead of
Explicit Register Variables.
- Putting text on the Explicit Register Variables menu page (instead
of the Global or Local subpages) is redundant, since any text that
describes the usage of Global or Local register variables will have to
be repeated on the individual subpages.
- Vague promises of features that may some day be available are not
useful in documentation (Eventually there may be a way of asking the
compiler to).
- Vague descriptions of things that are reported to work on certain
platforms are not useful (On the SPARC, there are reports that).
- Describing Local Register Variables as sometimes convenient for use
with the extended asm feature misses the point that this is in fact the
ONLY supported use for Local Register Variables.
- Unambiguous statements such as The following uses are explicitly
/not/ supported when describing things such as calling Basic asm
discourage people from attempting to misuse the feature.
ChangeLog:
2014-06-30 David Wohlferd d...@limegreensocks.com
* doc/extend.texi (Explicit Reg Vars): Rewrite and clarify.
* doc/implement-c.texi (Hints implementation): Use new name.
dw
Index: extend.texi
===
--- extend.texi (revision 210997)
+++ extend.texi (working copy)
@@ -6234,7 +6234,8 @@
* Extended Asm:: Inline assembler with operands.
* Constraints::Constraints for @code{asm} operands
* Asm Labels:: Specifying the assembler name to use for a C symbol.
-* Explicit Reg Vars:: Defining variables residing in specified registers.
+* Explicit Register Variables:: Defining variables residing in specified
+ registers.
* Size of an asm:: How GCC calculates the size of an @code{asm} block.
@end menu
@@ -6723,8 +6724,8 @@
list as many alternates as the @code{asm} statement allows, you will permit
the optimizers to produce the best possible code. If you must use a specific
register, but your Machine Constraints do not provide sufficient
-control to select the specific register you want, Local Reg Vars may provide
-a solution (@pxref{Local Reg Vars}).
+control to select the specific register you want, Local Register Variables
+may provide a solution (@pxref{Local Register Variables}).
@emph{cvariablename}
@*
@@ -6865,8 +6866,8 @@
method based on the current context. Input constraints may not begin with
either = or +. If you must use a specific register, but your Machine
Constraints do not provide sufficient control to select the specific
-register you want, Local Reg Vars may provide a solution
-(@pxref{Local Reg Vars}).
+register you want, Local Register Variables may provide a solution
+(@pxref{Local Register Variables}).
Input constraints can also be digits (for example, @code{0}). This indicates
that the specified input will be in the same place as the output constraint
@@ -6952,7 +6953,8 @@
Clobber descriptions may not in any way overlap with an input or output
operand. For example, you may not have an operand describing a register class
with one member when listing that register in the clobber list. Variables
-declared to live in specific registers (@pxref{Explicit Reg Vars}), and used
+declared to live in specific registers (@pxref{Explicit Register Variables}),
+and used
as @code{asm} input or output operands, must have no part mentioned in the
clobber description. In particular, there is no way to specify that input
operands get modified without also specifying them as output operands.
@@ -7290,7 +7292,7 @@
It does not make sense to use this feature with a non-static local
variable since such variables do not have assembler names. If you are
trying to put the variable in a particular register, see @ref{Explicit
-Reg Vars}. GCC presently accepts such code with a warning, but will
+Register Variables}. GCC presently accepts such code with a warning, but will
probably be changed to issue an error, rather than a warning, in the
future.
@@ -7312,197 +7314,128 @@
does not as yet have the ability to store static variables in registers.
Perhaps that will be added.
-@node Explicit Reg Vars
+@node Explicit Register Variables
@subsection Variables in Specified Registers
@cindex explicit register variables
@cindex variables in specified registers
@cindex specified registers
-@cindex registers, global allocation
-GNU C allows you to put a few global variables into specified hardware
-registers. You can also specify the register in which an ordinary
-register variable should be allocated.
+GNU C allows you to associate specific hardware registers
Re: Optimize type streaming
On June 29, 2014 9:53:03 PM CEST, Jan Hubicka hubi...@ucw.cz wrote:
In addition of pr61644 and pr61646, this commit breaks a lot of
fortran tests with -flto -O0.
Hello,
the problem here is that we have POINTER_TYPE that points to array of
variable
length (not sure why it happens only with -O0). The ICE is introduced
by the
fact that we stream the type inside of function body then and thus it
never
goes through the lto.c's type handling.
This patch fixes the ICE by moving the fixup somewhat earlier (perhaps
it is
good idea in general). I do not believe resulting IL is correct: we do
not
compute canonical type of the pointer nor we link correctly the
variants.
Richard, we probably ought to try to make all of the canonical type
fixup
happen also for function local types which would involve moving it all
from
lto.c into generic code? :((
I am testing the patch. I think it is better to have fixup here, so OK
if
it passes?
Please revert the original patch instead which was not tested properly. I'll
get back to this after I return from vacation.
Richard.
Honza
Index: lto-streamer-in.c
===
--- lto-streamer-in.c (revision 212098)
+++ lto-streamer-in.c (working copy)
@@ -1182,6 +1182,58 @@ lto_read_tree (struct lto_input_block *i
}
+/* Copy fields that are not streamed but copied from other nodes. */
+static void
+lto_copy_fields_not_streamed (tree t)
+{
+ if (TYPE_P (t) TYPE_MAIN_VARIANT (t) != t)
+{
+ tree mv = TYPE_MAIN_VARIANT (t);
+
+ if (COMPLETE_TYPE_P (t))
+ {
+TYPE_SIZE (t) = TYPE_SIZE (mv);
+TYPE_SIZE_UNIT (t) = TYPE_SIZE_UNIT (mv);
+ }
+ TYPE_ATTRIBUTES (t) = TYPE_ATTRIBUTES (mv);
+
+ if (CODE_CONTAINS_STRUCT (TREE_CODE (t), TS_TYPE_NON_COMMON))
+ {
+if (TREE_CODE (t) == ENUMERAL_TYPE COMPLETE_TYPE_P (t))
+ TYPE_VALUES (t) = TYPE_VALUES (mv);
+else if (TREE_CODE (t) == ARRAY_TYPE)
+ TYPE_DOMAIN (t) = TYPE_DOMAIN (mv);
+
+ if (RECORD_OR_UNION_TYPE_P (t) COMPLETE_TYPE_P (t))
+ TYPE_VFIELD (t) = TYPE_VFIELD (mv);
+else if ((TREE_CODE (t) == ENUMERAL_TYPE COMPLETE_TYPE_P (t))
+ || TREE_CODE (t) == INTEGER_TYPE
+ || TREE_CODE (t) == BOOLEAN_TYPE
+ || TREE_CODE (t) == REAL_TYPE
+ || TREE_CODE (t) == FIXED_POINT_TYPE)
+ TYPE_MIN_VALUE (t) = TYPE_MIN_VALUE (mv);
+
+if (TREE_CODE (t) == METHOD_TYPE)
+ TYPE_METHOD_BASETYPE (t) = TYPE_METHOD_BASETYPE (mv);
+else if (RECORD_OR_UNION_TYPE_P (t) COMPLETE_TYPE_P (t))
+ TYPE_METHODS (t) = TYPE_METHODS (mv);
+else if (TREE_CODE (t) == OFFSET_TYPE)
+ TYPE_OFFSET_BASETYPE (t) = TYPE_OFFSET_BASETYPE (mv);
+else if (TREE_CODE (t) == ARRAY_TYPE)
+ TYPE_ARRAY_MAX_SIZE (t) = TYPE_ARRAY_MAX_SIZE (mv);
+else if ((TREE_CODE (t) == ENUMERAL_TYPE COMPLETE_TYPE_P (t))
+ || TREE_CODE (t) == INTEGER_TYPE
+ || TREE_CODE (t) == BOOLEAN_TYPE
+ || TREE_CODE (t) == REAL_TYPE
+ || TREE_CODE (t) == FIXED_POINT_TYPE)
+ TYPE_MAX_VALUE (t) = TYPE_MAX_VALUE (mv);
+
+if (RECORD_OR_UNION_TYPE_P (t) COMPLETE_TYPE_P (t))
+ TYPE_BINFO (t) = TYPE_BINFO (mv);
+ }
+}
+}
+
/* Populate the reader cache with trees materialized from the SCC
following in the IB, DATA_IN stream. */
@@ -1194,6 +1246,7 @@ lto_input_scc (struct lto_input_block *i
unsigned size = streamer_read_uhwi (ib);
hashval_t scc_hash = streamer_read_uhwi (ib);
unsigned scc_entry_len = 1;
+ unsigned from = data_in-reader_cache-nodes.length ();
if (size == 1)
{
@@ -1233,6 +1286,12 @@ lto_input_scc (struct lto_input_block *i
}
}
+ /* Copy fileds we do not stream before unification so we can compare
them
+ without being worried if they are already initialized. */
+ for (unsigned i = 0; i size; ++i)
+lto_copy_fields_not_streamed
+ (streamer_tree_cache_get_tree (data_in-reader_cache, from +
i));
+
*len = size;
*entry_len = scc_entry_len;
return scc_hash;
Index: lto/lto.c
===
--- lto/lto.c (revision 212114)
+++ lto/lto.c (working copy)
@@ -1050,58 +1050,6 @@ lto_register_function_decl_in_symtab (st
decl, get_resolution (data_in, ix));
}
-/* Copy fields that are not streamed but copied from other nodes. */
-static void
-lto_copy_fields_not_streamed (tree t)
-{
- if (TYPE_P (t) TYPE_MAIN_VARIANT (t) != t)
-{
- tree mv = TYPE_MAIN_VARIANT (t);
-
- if (COMPLETE_TYPE_P (t))
- {
-TYPE_SIZE (t) = TYPE_SIZE (mv);
-TYPE_SIZE_UNIT (t) = TYPE_SIZE_UNIT (mv);
- }
- TYPE_ATTRIBUTES (t) = TYPE_ATTRIBUTES (mv);
-
- if (CODE_CONTAINS_STRUCT (TREE_CODE (t), TS_TYPE_NON_COMMON))
- {
-if (TREE_CODE
Re: [PATCH] Remove bswap STRICT_ALING target limitation
Richard Biener richard.guent...@gmail.com writes: On June 27, 2014 2:50:33 PM CEST, Rainer Orth r...@cebitec.uni-bielefeld.de wrote: Richard Biener rguent...@suse.de writes: The following patch enables bswap-from-load for STRICT_ALIGNMENT targets when the load is aligned instead of unconditionally (which probably was not intended). (note to self: we should fully transition to use SLOW_UNALIGNED_ACCESS) Bootstrapped and tested on x86_64-unknown-linux-gnu. Richard. 2014-06-27 Richard Biener rguent...@suse.de * tree-ssa-math-opts.c (bswap_replace): Fix SLOW_UNALIGNED_ACCESS test to only apply to unaligned object. This should fix PR bootstrap/61320. That would be odd. You were right, of course: I should have read more closely. Anyway, I'm again back to bootstrap land until Thomas comes up with a real patch. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: Optimize type streaming
Please revert the original patch instead which was not tested properly. I'll get back to this after I return from vacation. OK, I will bootstrap and revert tomorrow morning. Note that testusite passes with the patch; the problem appears only at -O0 -flto and fortran that we apparently do not test at all. It is bit odd that we do not have any testcases for VLA types at -O2? Honza
[wwwdocs,Java] Remove separate java/build-snapshot.html
When we integrated GCJ/libgcj, it looks like we did not proceed
doing that fully as it came to the web pages, and sadly nobody
else has been looking after them for a while.
2014-06-30 Gerald Pfeifer ger...@pfeifer.com
* build-snapshot.html: Remove.
* gcj2.html: Remove link to build-snapshot.html and GCC homepage.
* libgcj2.html: Replace link to build-snapshot.html by our main
installation instructions. Omit redundant instructions.
Applied.
Gerald
Index: gcj2.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/java/gcj2.html,v
retrieving revision 1.10
diff -u -r1.10 gcj2.html
--- gcj2.html 29 Dec 2012 17:29:24 - 1.10
+++ gcj2.html 30 Jun 2014 09:18:02 -
@@ -13,8 +13,7 @@
pWe've written a front end to the GCC compiler which can natively
compile both Javasupa href=#javatmtm/a/sup source and
bytecode files. The compiler can also generate class files. This new
-front end is integrated into the a
-href=http://gcc.gnu.org/;GCC/a project./p
+front end is integrated into GCC./p
h2What you get/h2
@@ -60,11 +59,7 @@
pIn order to make full executables, you'll need to link the
output of gcj with the appropriate runtime code. See a
-href=libgcj2.htmlthe libgcj page/a for details on the runtime.
-Note that you'll want to configure GCC to build libjava; see
-a href=build-snapshot.htmlconfiguration and build instructions
-for GCJ/a./p
-
+href=libgcj2.htmlthe libgcj page/a for details on the runtime./p
pThere are also a href=compile.htmlmore detailed
instructions/a on compiling Java programs./p
Index: libgcj2.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/java/libgcj2.html,v
retrieving revision 1.10
diff -u -r1.10 libgcj2.html
--- libgcj2.html17 Apr 2003 13:05:51 - 1.10
+++ libgcj2.html30 Jun 2014 09:18:02 -
@@ -31,32 +31,8 @@
h2How to build it/h2
-pJust follow the directions for
-a href=build-snapshot.htmlbuilding/a GCJ./p
-
-pThere are a few options you might consider passing to
-``configure'':/p
-
-dl
- dt--enable-java-gc=TYPE/dt
- ddEnables the garbage collector. The default type is
- ``boehm'', for the Boehm conservative GC./dd
-
- dt--enable-fast-character/dt
- ddlibgcj includes two implementations of java.lang.Character.
- The default implementation is small, but slow. This option will
- enable the faster, larger code./dd
-
- dt--enable-threads=TYPE/dt
- ddlibgcj includes a retargetable thread layer. The value of
- this option must be the same as the value used when building
- gcj itself.
- By default, no threads will be used (this is a crummy default,
- but we just follow GCC here).
- The only supported values are ``none'' (for no threads) and
- ``posix'' (for POSIX threads)./dd
-
-/dl
+pJust follow our a href=https://gcc.gnu.org/install/;installation
+instructions/a./p
/body
/html
Index: build-snapshot.html
===
RCS file: build-snapshot.html
diff -N build-snapshot.html
--- build-snapshot.html 28 Jun 2014 11:59:44 - 1.19
+++ /dev/null 1 Jan 1970 00:00:00 -
@@ -1,129 +0,0 @@
-html
-
-head
-titleHow to build GCJ/LIBGCJ from snapshots or checkouts/title
-/head
-
-body
-
-h1How to build and run libgcj/gcj snapshots or checkouts/h1
-
-table border=0 cellpadding=4 width=95%
-tr bgcolor=#cc
-td
-pre
-1. a href=../snapshots.htmlGet a GCC snapshot/a or a
href=../svn.htmlcheck out the sources/a.
-/pre
-/td
-/tr
-
-tr bgcolor=#dd
-td
-pre
-2. Make a compile directory
-
- $ mkdir compile
-/pre
-/td
-/tr
-
-tr bgcolor=#cc
-td
-pre
-3. Move the snapshot into the compile dir, e.g.
-
- $ cd compile
- $ mv ../gcc-20001211.tar.gz .
- $ gunzip *.gz
- $ tar xfv *.tar
- $ ln -s gcc-20001211 gcc
-/pre
-/td
-/tr
-
-tr bgcolor=#dd
-td
-pre
-4. Tell the build system you want to build libgcj
-
- Have a look at the toplevel configure.in (gcc/configure.in) and
- make sure that the variable `noconfigdirs' isn't assigned to
- something (like target-libjava or ${libgcj}.)
-br /
- Also, check for platform-specific assignments of `noconfigdirs' with
- ${libgcj}; if ${libgcj} is listed for your platform, remove it before
- configuring.
-/pre/td/tr
-
-tr bgcolor=#cc
-td
-pre
-5. Compile and install gcc/gcj/libgcj
-
- $ cd compile
- $ mkdir objdir
- $ cd objdir
- $ ../gcc/configure --enable-threads=posix --prefix=/home/joerg/gcc \
---enable-shared --enable-languages=c++,java \
---with-as=/opt/gnu/bin/as --with-ld=/opt/gnu/bin/ld
- $ make bootstrap
- $ make
- $ make install
-
-The Boehm GC is the collector picked up by default.
-
-If you compile under GNU/Linux you could omit the last two options. Under
-Solaris you'll need them. If you omit
Re: AARCH64 configure check for gas -mabi support
I applied the small patch on top of this, mostly triggered by the
markup issue.
Let me know if there is anything you'd like to see differently; I
am thinking to push back to GCC 4.9 as well later.
Gerald
2014-06-30 Gerald Pfeifer ger...@pfeifer.com
* doc/install.texi (Specific, aarch64*-*-*): Fix markup. Reword a bit.
Index: doc/install.texi
===
--- doc/install.texi(revision 212139)
+++ doc/install.texi(working copy)
@@ -3760,9 +3760,9 @@
@end html
@anchor{aarch64-x-x}
@heading aarch64*-*-*
-Pre 2.24 binutils does not have support for selecting -mabi and does not
-support ILP32. If GCC 4.9 or later is built with pre 2.24, GCC will not
-support option -mabi=ilp32.
+Binutils pre 2.24 does not have support for selecting @option{-mabi} and
+does not support ILP32. If it is used to build GCC 4.9 or later, GCC will
+not support option @option{-mabi=ilp32}.
@html
hr /
Re: AARCH64 configure check for gas -mabi support
Looks good to me. Thanks for the fix.
Yufeng
On 06/30/14 10:44, Gerald Pfeifer wrote:
I applied the small patch on top of this, mostly triggered by the
markup issue.
Let me know if there is anything you'd like to see differently; I
am thinking to push back to GCC 4.9 as well later.
Gerald
2014-06-30 Gerald Pfeiferger...@pfeifer.com
* doc/install.texi (Specific, aarch64*-*-*): Fix markup. Reword a bit.
Index: doc/install.texi
===
--- doc/install.texi(revision 212139)
+++ doc/install.texi(working copy)
@@ -3760,9 +3760,9 @@
@end html
@anchor{aarch64-x-x}
@heading aarch64*-*-*
-Pre 2.24 binutils does not have support for selecting -mabi and does not
-support ILP32. If GCC 4.9 or later is built with pre 2.24, GCC will not
-support option -mabi=ilp32.
+Binutils pre 2.24 does not have support for selecting @option{-mabi} and
+does not support ILP32. If it is used to build GCC 4.9 or later, GCC will
+not support option @option{-mabi=ilp32}.
@html
hr /
Re: [PATCH, rs6000, testsuite] Skip gfortran.dg/round_4.f90 for PowerPC Linux
Hi Bill, Bill Schmidt wrote: The test in gfortran.dg/round_4.f90, introduced in GCC 4.9, checks for correct behavior of different rounding modes. However, for quad-precision floating-point, it requires that the number 0.100481 be exactly represented. Since the PowerPC long double implementation (double-double) only guarantees 31 bits of precision, the test fails for the real(qp) portions of the test. Thus this patch marks the test invalid for PowerPC Linux for now. (We may want to also do this for other subtargets; let me know if so.) At such time as IEEE 128-bit floating-point is supported by the PowerPC port, we should revisit this. Is this ok for trunk and 4.9? Looks good to me. Thanks for digging into test-suite failures! However, it probably wouldn't harm either to add a test case for powerpc*-*-* (which might also work with hppa-*-*-hpux) for the rounding. I worry less about the quad-precision (qp) rounding, but I think it would be useful to test that the single and double precision rounding work. But as it requires an extra file, one could also check for the 31bit QP as well. Thus, if you have some spare cycles and agree on the usefulness ... Tobias 2014-06-29 Bill Schmidt wschm...@linux.vnet.ibm.com * gfortran.dg/round_4.f90: Skip for powerpc*-*-linux* since the test requires greater precision than the current PowerPC long double implementation supports.
Re: [PATCH][AArch64] Fix some saturating math NEON intrinsics types
On 30 June 2014 10:41, Kyrill Tkachov kyrylo.tkac...@arm.com wrote: Here it is. Now it applies cleanly to 4.9 and the tests are in gcc.target/aarch64 instead of gcc.target/aarch64/simd Tested on aarch64-none-elf and aarch64_be-none-elf. Ok to apply? OK, thanks. /Marcus
Re: [PATCH][AArch64] Fix some saturating math NEON intrinsics types
On 30/06/14 11:44, Marcus Shawcroft wrote: On 30 June 2014 10:41, Kyrill Tkachov kyrylo.tkac...@arm.com wrote: Here it is. Now it applies cleanly to 4.9 and the tests are in gcc.target/aarch64 instead of gcc.target/aarch64/simd Tested on aarch64-none-elf and aarch64_be-none-elf. Ok to apply? OK, thanks. /Marcus Thanks, committed with r212141. Kyrill
Re: [Patch ARM/testsuite 03/22] Add binary operators: vadd, vand, vbic, veor, vorn, vorr, vsub.
On 30 June 2014 09:03, Ramana Radhakrishnan ramana@googlemail.com wrote: + Move the tests to gcc.target/arm/ to gcc.target/aarch64 if the AArch64 maintainers agree. For the extra AArch64 variants guard them with #ifdef __aarch64__ #endif. Given that the intrinsics in aarch64 are a superset of those in aarch32 I agree that these tests would be better located under the aarch64 tree. /Marcus
Re: [commit] Fix ABI fallout (Re: wide-int, rs6000)
Mike Stump wrote: On Jun 28, 2014, at 3:31 AM, Ulrich Weigand uweig...@de.ibm.com wrote: Mike Stump wrote: (rs6000_aggregate_candidate): Use wide-int interfaces. [snip] - /* Can't handle incomplete types. */ - if (!COMPLETE_TYPE_P (type)) -return -1; + /* Can't handle incomplete types nor sizes that are not + fixed. */ + if (!COMPLETE_TYPE_P (type) + || TREE_CODE (TYPE_SIZE (type)) != INTEGER_CST) Ouch, sorry. At least 8 eyes missed it. Does this fix the -m32 issue? I'm not sure what -m32 issue you're refering to ... This bug was in rs6000_aggregate_candidate, which is only used with the ELFv2 ABI, which we currently support on 64-bit only. Bye, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain ulrich.weig...@de.ibm.com
Re: [PATCH 2/5] Existing call graph infrastructure enhancement
On 06/17/2014 10:00 PM, Jeff Law wrote:
On 06/13/14 04:26, mliska wrote:
Hi,
this small patch prepares remaining needed infrastructure for the new pass.
Changelog:
2014-06-13 Martin Liska mli...@suse.cz
Honza Hubicka hubi...@ucw.cz
* ipa-utils.h (polymorphic_type_binfo_p): Function marked external
instead of static.
* ipa-devirt.c (polymorphic_type_binfo_p): Likewise.
* ipa-prop.h (count_formal_params): Likewise.
* ipa-prop.c (count_formal_params): Likewise.
* ipa-utils.c (ipa_merge_profiles): Be more tolerant if we merge
profiles for semantically equivalent functions.
* passes.c (do_per_function): If we load body of a function during WPA,
this condition should behave same.
* varpool.c (ctor_for_folding): More tolerant assert for variable
aliases created during WPA.
Presumably we don't have any useful way to merge the cases where we have provides
for SRC DST in ipa_merge_profiles or even to guess which is more useful when
presented with both? Does it make sense to log this into a debugging file when we
drop one?
Hello,
this merge function was written by Honza, what do you think Honza about this
note?
I think this patch is fine. If adding logging makes sense, then feel free to
do so and consider that trivial change pre-approved.
I made a small change to this patch, where I moved 'gsi_next_nonvirtual_phi'
from the pass to gimple-iterator.h.
Ready for trunk with this change?
Thanks,
Martin
gcc/ChangeLog
2014-06-30 Martin Liska mli...@suse.cz
Honza Hubicka hubi...@ucw.cz
* gimple-iterator.h (gsi_next_nonvirtual_phi): New function.
* ipa-utils.h (polymorphic_type_binfo_p): Function marked external
instead of static.
* ipa-devirt.c (polymorphic_type_binfo_p): Likewise.
* ipa-prop.h (count_formal_params): Likewise.
* ipa-prop.c (count_formal_params): Likewise.
* ipa-utils.c (ipa_merge_profiles): Be more tolerant if we merge
profiles for semantically equivalent functions.
* passes.c (do_per_function): If we load body of a function during WPA,
this condition should behave same.
* varpool.c (ctor_for_folding): More tolerant assert for variable
aliases created during WPA.
Jeff
diff --git a/gcc/gimple-iterator.h b/gcc/gimple-iterator.h
index 909d58b..47168b9 100644
--- a/gcc/gimple-iterator.h
+++ b/gcc/gimple-iterator.h
@@ -281,6 +281,30 @@ gsi_last_nondebug_bb (basic_block bb)
return i;
}
+/* Iterates I statement iterator to the next non-virtual statement. */
+
+static inline void
+gsi_next_nonvirtual_phi (gimple_stmt_iterator *i)
+{
+ gimple phi;
+
+ if (gsi_end_p (*i))
+return;
+
+ phi = gsi_stmt (*i);
+ gcc_assert (phi != NULL);
+
+ while (virtual_operand_p (gimple_phi_result (phi)))
+{
+ gsi_next (i);
+
+ if (gsi_end_p (*i))
+ return;
+
+ phi = gsi_stmt (*i);
+}
+}
+
/* Return the basic block associated with this iterator. */
static inline basic_block
diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index d6b85bf..2f62323 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -176,7 +176,7 @@ struct GTY(()) odr_type_d
inheritance (because vtables are shared). Look up the BINFO of type
and check presence of its vtable. */
-static inline bool
+bool
polymorphic_type_binfo_p (tree binfo)
{
/* See if BINFO's type has an virtual table associtated with it. */
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 68efc77..bb2fbf3 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -210,7 +210,7 @@ ipa_populate_param_decls (struct cgraph_node *node,
/* Return how many formal parameters FNDECL has. */
-static inline int
+int
count_formal_params (tree fndecl)
{
tree parm;
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 8886e93..bc6249e 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -529,6 +529,7 @@ void ipa_free_all_edge_args (void);
void ipa_free_all_structures_after_ipa_cp (void);
void ipa_free_all_structures_after_iinln (void);
void ipa_register_cgraph_hooks (void);
+int count_formal_params (tree fndecl);
/* This function ensures the array of node param infos is big enough to
accommodate a structure for all nodes and reallocates it if not. */
diff --git a/gcc/ipa-utils.c b/gcc/ipa-utils.c
index c191210..d58b170 100644
--- a/gcc/ipa-utils.c
+++ b/gcc/ipa-utils.c
@@ -660,13 +660,8 @@ ipa_merge_profiles (struct cgraph_node *dst,
if (dst-tp_first_run src-tp_first_run src-tp_first_run)
dst-tp_first_run = src-tp_first_run;
- if (src-profile_id)
-{
- if (!dst-profile_id)
- dst-profile_id = src-profile_id;
- else
- gcc_assert (src-profile_id == dst-profile_id);
-}
+ if (src-profile_id !dst-profile_id)
+dst-profile_id = src-profile_id;
if (!dst-count)
return;
diff --git a/gcc/ipa-utils.h b/gcc/ipa-utils.h
index a2c985a..996249a 100644
--- a/gcc/ipa-utils.h
+++ b/gcc/ipa-utils.h
@@ -72,6 +72,8 @@ struct odr_type_d;
typedef odr_type_d *odr_type;
[PATCH] Properly honor no_sanitize_undefined attribute
Apparently I didn't pay much attention to no_sanitize_undefined
attribute when adding new features to ubsan, so we would instrument
stuff even though the function is marked with this attribute.
Thus fixed new testcases added.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2014-06-30 Marek Polacek pola...@redhat.com
* convert.c (convert_to_integer): Don't instrument conversions if the
function has no_sanitize_undefined attribute.
* ubsan.c: Don't run the ubsan pass if the function has
no_sanitize_undefined attribute.
c/
* c-decl.c (grokdeclarator): Don't instrument VLAs if the function
has no_sanitize_undefined attribute.
cp/
* cp-gimplify.c (cp_genericize): Don't instrument returns if the
function has no_sanitize_undefined attribute.
* decl.c (compute_array_index_type): Don't instrument VLAs if the
function has no_sanitize_undefined attribute.
testsuite/
* c-c++-common/ubsan/attrib-2.c: New test.
* g++.dg/ubsan/return-3.C: New test.
diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index def10a2..7895510 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -5505,7 +5505,11 @@ grokdeclarator (const struct c_declarator *declarator,
this_size_varies = size_varies = true;
warn_variable_length_array (name, size);
if (flag_sanitize SANITIZE_VLA
-decl_context == NORMAL)
+decl_context == NORMAL
+current_function_decl != 0
+!lookup_attribute (no_sanitize_undefined,
+ DECL_ATTRIBUTES
+ (current_function_decl)))
{
/* Evaluate the array size only once. */
size = c_save_expr (size);
diff --git gcc/convert.c gcc/convert.c
index 2d9600d..dbb59da 100644
--- gcc/convert.c
+++ gcc/convert.c
@@ -847,7 +847,10 @@ convert_to_integer (tree type, tree expr)
return build1 (CONVERT_EXPR, type, expr);
case REAL_TYPE:
- if (flag_sanitize SANITIZE_FLOAT_CAST)
+ if (flag_sanitize SANITIZE_FLOAT_CAST
+ current_function_decl != 0
+ !lookup_attribute (no_sanitize_undefined,
+ DECL_ATTRIBUTES (current_function_decl)))
{
expr = save_expr (expr);
tree check = ubsan_instrument_float_cast (loc, type, expr);
diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c
index 296bd5f..dfa954b 100644
--- gcc/cp/cp-gimplify.c
+++ gcc/cp/cp-gimplify.c
@@ -1221,7 +1221,7 @@ cp_genericize_tree (tree* t_p)
/* If a function that should end with a return in non-void
function doesn't obviously end with return, add ubsan
- instrmentation code to verify it at runtime. */
+ instrumentation code to verify it at runtime. */
static void
cp_ubsan_maybe_instrument_return (tree fndecl)
@@ -1334,7 +1334,10 @@ cp_genericize (tree fndecl)
walk_tree's hash functionality. */
cp_genericize_tree (DECL_SAVED_TREE (fndecl));
- if (flag_sanitize SANITIZE_RETURN)
+ if (flag_sanitize SANITIZE_RETURN
+ current_function_decl != 0
+ !lookup_attribute (no_sanitize_undefined,
+ DECL_ATTRIBUTES (current_function_decl)))
cp_ubsan_maybe_instrument_return (fndecl);
/* Do everything else. */
diff --git gcc/cp/decl.c gcc/cp/decl.c
index d548f61..861e7a8 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -8469,7 +8469,11 @@ compute_array_index_type (tree name, tree size,
tsubst_flags_t complain)
throw_bad_array_length (), void_node);
finish_expr_stmt (comp);
}
- else if (flag_sanitize SANITIZE_VLA)
+ else if (flag_sanitize SANITIZE_VLA
+ current_function_decl != 0
+ !lookup_attribute (no_sanitize_undefined,
+DECL_ATTRIBUTES
+ (current_function_decl)))
{
/* From C++1y onwards, we throw an exception on a negative
length size of an array; see above. */
diff --git gcc/testsuite/c-c++-common/ubsan/attrib-2.c
gcc/testsuite/c-c++-common/ubsan/attrib-2.c
index e69de29..71f2e58 100644
--- gcc/testsuite/c-c++-common/ubsan/attrib-2.c
+++ gcc/testsuite/c-c++-common/ubsan/attrib-2.c
@@ -0,0 +1,71 @@
+/* { dg-do compile } */
+/* { dg-options -fsanitize=undefined -fsanitize=float-divide-by-zero
-fsanitize=float-cast-overflow } */
+
+/* Test that we don't instrument functions marked with
+ no_sanitize_undefined attribute. */
+
+#ifndef __cplusplus
+#define bool _Bool
+#endif
+enum A { B = -3, C = 2 } a;
+bool b;
+
+__attribute__((no_sanitize_undefined))
+static void
+vla_bound (void)
+{
+ int i = -1;
+ volatile int a[i];
+}
+
+__attribute__((no_sanitize_undefined))
+static void
+si_overflow
Re: [PATCH 4/5] Existing tests fix
On 06/17/2014 09:52 PM, Jeff Law wrote:
On 06/13/14 04:48, mliska wrote:
Hi,
many tests rely on a precise number of scanned functions in a dump file. If
IPA ICF decides to merge some function and(or) read-only variables, counts do
not match.
Martin
Changelog:
2014-06-13 Martin Liska mli...@suse.cz
Honza Hubicka hubi...@ucw.cz
* c-c++-common/rotate-1.c: Text
* c-c++-common/rotate-2.c: New test.
* c-c++-common/rotate-3.c: Likewise.
* c-c++-common/rotate-4.c: Likewise.
* g++.dg/cpp0x/rv-return.C: Likewise.
* g++.dg/cpp0x/rv1n.C: Likewise.
* g++.dg/cpp0x/rv1p.C: Likewise.
* g++.dg/cpp0x/rv2n.C: Likewise.
* g++.dg/cpp0x/rv3n.C: Likewise.
* g++.dg/cpp0x/rv4n.C: Likewise.
* g++.dg/cpp0x/rv5n.C: Likewise.
* g++.dg/cpp0x/rv6n.C: Likewise.
* g++.dg/cpp0x/rv7n.C: Likewise.
* gcc.dg/ipa/ipacost-1.c: Likewise.
* gcc.dg/ipa/ipacost-2.c: Likewise.
* gcc.dg/ipa/ipcp-agg-6.c: Likewise.
* gcc.dg/ipa/remref-2a.c: Likewise.
* gcc.dg/ipa/remref-2b.c: Likewise.
* gcc.dg/pr46309-2.c: Likewise.
* gcc.dg/torture/ipa-pta-1.c: Likewise.
* gcc.dg/tree-ssa/andor-3.c: Likewise.
* gcc.dg/tree-ssa/andor-4.c: Likewise.
* gcc.dg/tree-ssa/andor-5.c: Likewise.
* gcc.dg/vect/no-vfa-pr29145.c: Likewise.
* gcc.dg/vect/vect-cond-10.c: Likewise.
* gcc.dg/vect/vect-cond-9.c: Likewise.
* gcc.dg/vect/vect-widen-mult-const-s16.c: Likewise.
* gcc.dg/vect/vect-widen-mult-const-u16.c: Likewise.
* gcc.dg/vect/vect-widen-mult-half-u8.c: Likewise.
* gcc.target/i386/bmi-1.c: Likewise.
* gcc.target/i386/bmi-2.c: Likewise.
* gcc.target/i386/pr56564-2.c: Likewise.
* g++.dg/opt/pr30965.C: Likewise.
* g++.dg/tree-ssa/pr19637.C: Likewise.
* gcc.dg/guality/csttest.c: Likewise.
* gcc.dg/ipa/iinline-4.c: Likewise.
* gcc.dg/ipa/iinline-7.c: Likewise.
* gcc.dg/ipa/ipa-pta-13.c: Likewise.
I know this is the least interesting part of your changes, but it's also simple
and mechanical and thus trivial to review. Approved, but obviously don't
install until the rest of your patch has been approved.
Similar changes for recently added tests or cases where you might improve ICF
requiring similar tweaks to existing tests are pre-approved as well.
jeff
Hello,
I fixed few more tests and added correct ChangeLog message.
gcc/testsuite/ChangeLog
2014-06-30 Martin Liska mli...@suse.cz
Honza Hubicka hubi...@ucw.cz
* c-c++-common/rotate-1.c: Test fixed.
* c-c++-common/rotate-2.c: Likewise.
* c-c++-common/rotate-3.c: Likewise.
* c-c++-common/rotate-4.c: Likewise.
* g++.dg/cpp0x/rv-return.C: Likewise.
* g++.dg/cpp0x/rv1n.C: Likewise.
* g++.dg/cpp0x/rv1p.C: Likewise.
* g++.dg/cpp0x/rv2n.C: Likewise.
* g++.dg/cpp0x/rv3n.C: Likewise.
* g++.dg/cpp0x/rv4n.C: Likewise.
* g++.dg/cpp0x/rv5n.C: Likewise.
* g++.dg/cpp0x/rv6n.C: Likewise.
* g++.dg/cpp0x/rv7n.C: Likewise.
* g++.dg/ipa/devirt-g-1.C: Likewise.
* g++.dg/ipa/inline-1.C: Likewise.
* g++.dg/ipa/inline-2.C: Likewise.
* g++.dg/ipa/inline-3.C: Likewise.
* g++.dg/opt/pr30965.C: Likewise.
* g++.dg/tree-ssa/pr19637.C: Likewise.
* gcc.dg/guality/csttest.c: Likewise.
* gcc.dg/ipa/iinline-4.c: Likewise.
* gcc.dg/ipa/iinline-7.c: Likewise.
* gcc.dg/ipa/ipa-pta-13.c: Likewise.
* gcc.dg/ipa/ipacost-1.c: Likewise.
* gcc.dg/ipa/ipacost-2.c: Likewise.
* gcc.dg/ipa/ipcp-agg-6.c: Likewise.
* gcc.dg/ipa/remref-2a.c: Likewise.
* gcc.dg/ipa/remref-2b.c: Likewise.
* gcc.dg/pr46309-2.c: Likewise.
* gcc.dg/torture/ipa-pta-1.c: Likewise.
* gcc.dg/tree-ssa/andor-3.c: Likewise.
* gcc.dg/tree-ssa/andor-4.c: Likewise.
* gcc.dg/tree-ssa/andor-5.c: Likewise.
* gcc.dg/vect/no-vfa-pr29145.c: Likewise.
* gcc.dg/vect/vect-cond-10.c: Likewise.
* gcc.dg/vect/vect-cond-9.c: Likewise.
* gcc.dg/vect/vect-widen-mult-const-s16.c: Likewise.
* gcc.dg/vect/vect-widen-mult-const-u16.c: Likewise.
* gcc.dg/vect/vect-widen-mult-half-u8.c: Likewise.
* gcc.target/i386/bmi-1.c: Likewise.
* gcc.target/i386/bmi-2.c: Likewise.
* gcc.target/i386/pr56564-2.c: Likewise.
Thank you,
Martin
diff --git a/gcc/testsuite/c-c++-common/rotate-1.c b/gcc/testsuite/c-c++-common/rotate-1.c
index afdaa28..bca9dd8 100644
--- a/gcc/testsuite/c-c++-common/rotate-1.c
+++ b/gcc/testsuite/c-c++-common/rotate-1.c
@@ -1,6 +1,6 @@
/* Check rotate pattern detection. */
/* { dg-do compile } */
-/* { dg-options -O2 -fdump-tree-optimized } */
+/* { dg-options -O2 -fno-ipa-icf -fdump-tree-optimized } */
/* { dg-final { scan-tree-dump-times r\[]\[] 96 optimized } } */
/* { dg-final { cleanup-tree-dump optimized } } */
diff --git a/gcc/testsuite/c-c++-common/rotate-2.c b/gcc/testsuite/c-c++-common/rotate-2.c
index 109fd32..4ffa218 100644
--- a/gcc/testsuite/c-c++-common/rotate-2.c
+++ b/gcc/testsuite/c-c++-common/rotate-2.c
@@ -1,6 +1,6 @@
/* Check
Re: [PATCH 5/5] New tests introduction
On 06/17/2014 09:53 PM, Jeff Law wrote:
On 06/13/14 05:16, mliska wrote:
Hi,
this is a new collection of tests for IPA ICF pass.
Martin
Changelog:
2014-06-13 Martin Liska mli...@suse.cz
Honza Hubicka hubi...@ucw.cz
* gcc/testsuite/g++.dg/ipa/ipa-se-1.C: New test.
* gcc/testsuite/g++.dg/ipa/ipa-se-2.C: Likewise.
* gcc/testsuite/g++.dg/ipa/ipa-se-3.C: Likewise.
* gcc/testsuite/g++.dg/ipa/ipa-se-4.C: Likewise.
* gcc/testsuite/g++.dg/ipa/ipa-se-5.C: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-1.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-10.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-11.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-12.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-13.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-14.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-15.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-16.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-17.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-18.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-19.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-2.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-20.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-21.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-22.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-23.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-24.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-25.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-26.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-27.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-28.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-3.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-4.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-5.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-6.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-7.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-8.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-se-9.c: Likewise.
Also approved, but please don't install entire the entire kit is approved.
I'd like to applaud you and Jan for including a nice baseline of tests.
jeff
Hi,
there's updatd baseline of tests.
Martin
gcc/testsuite/ChangeLog:
2014-06-30 Martin Liska mli...@suse.cz
Honza Hubicka hubi...@ucw.cz
* gcc/testsuite/g++.dg/ipa/ipa-icf-1.C: New test.
* gcc/testsuite/g++.dg/ipa/ipa-icf-2.C: Likewise.
* gcc/testsuite/g++.dg/ipa/ipa-icf-3.C: Likewise.
* gcc/testsuite/g++.dg/ipa/ipa-icf-4.C: Likewise.
* gcc/testsuite/g++.dg/ipa/ipa-icf-5.C: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-1.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-10.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-11.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-12.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-13.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-14.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-15.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-16.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-17.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-18.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-19.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-2.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-20.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-21.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-22.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-23.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-24.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-25.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-27.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-28.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-3.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-4.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-5.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-6.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-7.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-8.c: Likewise.
* gcc/testsuite/gcc.dg/ipa/ipa-icf-9.c: Likewise.
Martin
diff --git a/gcc/testsuite/g++.dg/ipa/ipa-icf-1.C b/gcc/testsuite/g++.dg/ipa/ipa-icf-1.C
new file mode 100644
index 000..d27abf4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/ipa-icf-1.C
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options -O2 -fdump-ipa-icf } */
+
+class A
+{
+public:
+ __attribute__ ((noinline))
+ virtual int Foo2()
+ {
+return v;
+ }
+
+ float f;
+ int v;
+};
+
+class B
+{
+public:
+ __attribute__ ((noinline))
+ int Bar2()
+ {
+return v;
+ }
+
+ float f, aaa;
+ int v;
+};
+
+int main()
+{
+ A a;
+ B b;
+
+ a.Foo2();
+ b.Bar2();
+
+ return 12345;
+}
+
+/* { dg-final { scan-ipa-dump-not Semantic equality hit: icf } } */
+/* { dg-final { scan-ipa-dump Equal symbols: 0 icf } } */
+/* { dg-final { cleanup-ipa-dump icf } } */
diff --git a/gcc/testsuite/g++.dg/ipa/ipa-icf-2.C
[PING*3][PATCH RTL] Extend mode-switching to support toggle
Hello, I still miss an approval for the middle-end part of http://gcc.gnu.org/ml/gcc-patches/2014-06/msg01038.html thanks, Christian
Re: [C++] Avoid producing duplicate binfos for member pointers
Hmm, I wonder why we need BINFOs at all for the ptrmemfunc RECORD_TYPEs, which are not classes. Jason
Re: [PATCH] [ARM] [RFC] Fix longstanding push_minipool_fix ICE (PR49423, lp1296601)
On 18 June 2014 00:02, Ramana Radhakrishnan ramana@googlemail.com wrote: Interesting workaround but can we investigate further how to fix this at the source rather than working around in the backend in this form. It's still a kludge that we carry in the backend rather than fix the problem at it's source. I'd rather try to fix the problem at the source rather than working around this in the backend. I still think this is a back-end bug. Unless I've missed something, it looks like the compiler has generated an insn which meets its constraints (we'd see an insn does not satisfy its constraints ICE if not) but the back-end generates an ICE much later, when trying to emit code for it. The problem with trying to fix the bug at source in reload is that this inconsistency will remain as a potential latent bug. I see two options to fix it - one is to teach the back-end to successfully generate code for this insn, and the other is to teach the back-end that such an insn is not valid. My proposed patch does the former. The latter can presumably be achieved by providing a different kind of memory constraint which disallows constant pool references for these insns although I haven't tried this yet. Charles
[committed] Fix up -fsanitize=bounds documentation
Hi!
-fsanitize=bounds is one of the -fsanitize=undefined suboptions, therefore
it should be documented as such.
Fixed thusly, committed to trunk as obvious.
2014-06-30 Jakub Jelinek ja...@redhat.com
* doc/invoke.texi (-fsanitize=bounds): Move to the table with
-fsanitize=undefined suboptions.
--- gcc/doc/invoke.texi.jj 2014-06-30 09:28:48.0 +0200
+++ gcc/doc/invoke.texi 2014-06-30 14:18:01.203027487 +0200
@@ -5440,6 +5440,13 @@ signed char a = SCHAR_MAX;
a++;
@end smallexample
+@item -fsanitize=bounds
+@opindex fsanitize=bounds
+
+This option enables instrumentation of array bounds. Various out of bounds
+accesses are detected. Flexible array members are not instrumented, as well
+as initializers of variables with static storage.
+
@end table
While @option{-ftrapv} causes traps for signed overflows to be emitted,
@@ -5461,13 +5468,6 @@ This option enables floating-point type
We check that the result of the conversion does not overflow.
This option does not work well with @code{FE_INVALID} exceptions enabled.
-@item -fsanitize=bounds
-@opindex fsanitize=bounds
-
-This option enables instrumentation of array bounds. Various out of bounds
-accesses are detected. Flexible array members are not instrumented, as well
-as initializers of variables with static storage.
-
@item -fsanitize-recover
@opindex fsanitize-recover
By default @option{-fsanitize=undefined} sanitization (and its suboptions
Jakub
Re: [PATCH] Properly honor no_sanitize_undefined attribute
On Mon, Jun 30, 2014 at 02:08:26PM +0200, Marek Polacek wrote: Apparently I didn't pay much attention to no_sanitize_undefined attribute when adding new features to ubsan, so we would instrument stuff even though the function is marked with this attribute. Thus fixed new testcases added. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2014-06-30 Marek Polacek pola...@redhat.com * convert.c (convert_to_integer): Don't instrument conversions if the function has no_sanitize_undefined attribute. * ubsan.c: Don't run the ubsan pass if the function has no_sanitize_undefined attribute. c/ * c-decl.c (grokdeclarator): Don't instrument VLAs if the function has no_sanitize_undefined attribute. cp/ * cp-gimplify.c (cp_genericize): Don't instrument returns if the function has no_sanitize_undefined attribute. * decl.c (compute_array_index_type): Don't instrument VLAs if the function has no_sanitize_undefined attribute. testsuite/ * c-c++-common/ubsan/attrib-2.c: New test. * g++.dg/ubsan/return-3.C: New test. Ok, thanks. Jakub
Re: [PATCH] Properly honor no_sanitize_undefined attribute
On Mon, Jun 30, 2014 at 02:58:53PM +0200, Jakub Jelinek wrote: On Mon, Jun 30, 2014 at 02:08:26PM +0200, Marek Polacek wrote: Apparently I didn't pay much attention to no_sanitize_undefined attribute when adding new features to ubsan, so we would instrument stuff even though the function is marked with this attribute. Thus fixed new testcases added. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2014-06-30 Marek Polacek pola...@redhat.com * convert.c (convert_to_integer): Don't instrument conversions if the function has no_sanitize_undefined attribute. * ubsan.c: Don't run the ubsan pass if the function has no_sanitize_undefined attribute. c/ * c-decl.c (grokdeclarator): Don't instrument VLAs if the function has no_sanitize_undefined attribute. cp/ * cp-gimplify.c (cp_genericize): Don't instrument returns if the function has no_sanitize_undefined attribute. * decl.c (compute_array_index_type): Don't instrument VLAs if the function has no_sanitize_undefined attribute. testsuite/ * c-c++-common/ubsan/attrib-2.c: New test. * g++.dg/ubsan/return-3.C: New test. Ok, thanks. Actually, please change current_function_decl != 0 to current_function_decl != NULL_TREE everywhere in the patch. Ok with that change. Jakub
Re: [PATCH v2] typeof: Remove type qualifiers for atomic types
On Fri, Jun 27, 2014 at 08:49:24AM +0200, Marek Polacek wrote: On Fri, Jun 27, 2014 at 08:36:38AM +0200, Sebastian Huber wrote: Thanks for your patience. It would be nice if this can be applied to GCC 4.9 as well. Can someone please commit this for me, since I don't have write access. I will commit it for you. Now applied to 4.9 as well. Marek
Re: [PATCH] Properly honor no_sanitize_undefined attribute
On Mon, Jun 30, 2014 at 03:00:11PM +0200, Jakub Jelinek wrote:
Actually, please change current_function_decl != 0 to
current_function_decl != NULL_TREE everywhere in the patch.
Ok with that change.
Ok, I'm applying the following then.
2014-06-30 Marek Polacek pola...@redhat.com
* convert.c (convert_to_integer): Don't instrument conversions if the
function has no_sanitize_undefined attribute.
* ubsan.c: Don't run the ubsan pass if the function has
no_sanitize_undefined attribute.
c/
* c-decl.c (grokdeclarator): Don't instrument VLAs if the function
has no_sanitize_undefined attribute.
cp/
* cp-gimplify.c (cp_genericize): Don't instrument returns if the
function has no_sanitize_undefined attribute.
* decl.c (compute_array_index_type): Don't instrument VLAs if the
function has no_sanitize_undefined attribute.
testsuite/
* c-c++-common/ubsan/attrib-2.c: New test.
* g++.dg/ubsan/return-3.C: New test.
diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index def10a2..7c37edf 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -5505,7 +5505,11 @@ grokdeclarator (const struct c_declarator *declarator,
this_size_varies = size_varies = true;
warn_variable_length_array (name, size);
if (flag_sanitize SANITIZE_VLA
-decl_context == NORMAL)
+decl_context == NORMAL
+current_function_decl != NULL_TREE
+!lookup_attribute (no_sanitize_undefined,
+ DECL_ATTRIBUTES
+ (current_function_decl)))
{
/* Evaluate the array size only once. */
size = c_save_expr (size);
diff --git gcc/convert.c gcc/convert.c
index 2d9600d..09bc555 100644
--- gcc/convert.c
+++ gcc/convert.c
@@ -847,7 +847,10 @@ convert_to_integer (tree type, tree expr)
return build1 (CONVERT_EXPR, type, expr);
case REAL_TYPE:
- if (flag_sanitize SANITIZE_FLOAT_CAST)
+ if (flag_sanitize SANITIZE_FLOAT_CAST
+ current_function_decl != NULL_TREE
+ !lookup_attribute (no_sanitize_undefined,
+ DECL_ATTRIBUTES (current_function_decl)))
{
expr = save_expr (expr);
tree check = ubsan_instrument_float_cast (loc, type, expr);
diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c
index 296bd5f..a35177b 100644
--- gcc/cp/cp-gimplify.c
+++ gcc/cp/cp-gimplify.c
@@ -1221,7 +1221,7 @@ cp_genericize_tree (tree* t_p)
/* If a function that should end with a return in non-void
function doesn't obviously end with return, add ubsan
- instrmentation code to verify it at runtime. */
+ instrumentation code to verify it at runtime. */
static void
cp_ubsan_maybe_instrument_return (tree fndecl)
@@ -1334,7 +1334,10 @@ cp_genericize (tree fndecl)
walk_tree's hash functionality. */
cp_genericize_tree (DECL_SAVED_TREE (fndecl));
- if (flag_sanitize SANITIZE_RETURN)
+ if (flag_sanitize SANITIZE_RETURN
+ current_function_decl != NULL_TREE
+ !lookup_attribute (no_sanitize_undefined,
+ DECL_ATTRIBUTES (current_function_decl)))
cp_ubsan_maybe_instrument_return (fndecl);
/* Do everything else. */
diff --git gcc/cp/decl.c gcc/cp/decl.c
index d548f61..6902bb0 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -8469,7 +8469,11 @@ compute_array_index_type (tree name, tree size,
tsubst_flags_t complain)
throw_bad_array_length (), void_node);
finish_expr_stmt (comp);
}
- else if (flag_sanitize SANITIZE_VLA)
+ else if (flag_sanitize SANITIZE_VLA
+ current_function_decl != NULL_TREE
+ !lookup_attribute (no_sanitize_undefined,
+DECL_ATTRIBUTES
+ (current_function_decl)))
{
/* From C++1y onwards, we throw an exception on a negative
length size of an array; see above. */
diff --git gcc/testsuite/c-c++-common/ubsan/attrib-2.c
gcc/testsuite/c-c++-common/ubsan/attrib-2.c
index e69de29..71f2e58 100644
--- gcc/testsuite/c-c++-common/ubsan/attrib-2.c
+++ gcc/testsuite/c-c++-common/ubsan/attrib-2.c
@@ -0,0 +1,71 @@
+/* { dg-do compile } */
+/* { dg-options -fsanitize=undefined -fsanitize=float-divide-by-zero
-fsanitize=float-cast-overflow } */
+
+/* Test that we don't instrument functions marked with
+ no_sanitize_undefined attribute. */
+
+#ifndef __cplusplus
+#define bool _Bool
+#endif
+enum A { B = -3, C = 2 } a;
+bool b;
+
+__attribute__((no_sanitize_undefined))
+static void
+vla_bound (void)
+{
+ int i = -1;
+ volatile int a[i];
+}
+
+__attribute__((no_sanitize_undefined))
+static void
+si_overflow (void)
+{
Re: [PATCH] [ARM] [RFC] Fix longstanding push_minipool_fix ICE (PR49423, lp1296601)
On 30/06/14 13:53, Charles Baylis wrote: On 18 June 2014 00:02, Ramana Radhakrishnan ramana@googlemail.com wrote: Interesting workaround but can we investigate further how to fix this at the source rather than working around in the backend in this form. It's still a kludge that we carry in the backend rather than fix the problem at it's source. I'd rather try to fix the problem at the source rather than working around this in the backend. I still think this is a back-end bug. Unless I've missed something, it looks like the compiler has generated an insn which meets its constraints (we'd see an insn does not satisfy its constraints ICE if not) but the back-end generates an ICE much later, when trying to emit code for it. The problem with trying to fix the bug at source in reload is that this inconsistency will remain as a potential latent bug. I see two options to fix it - one is to teach the back-end to successfully generate code for this insn, and the other is to teach the back-end that such an insn is not valid. My proposed patch does the former. The latter can presumably be achieved by providing a different kind of memory constraint which disallows constant pool references for these insns although I haven't tried this yet. Charles I think we should be doing the latter (not permitting these operations). If we wanted to do the former, we could just add an offset range for the insn. The reason we don't want the former is that the offset ranges are too small and overly constrain literal pool placement. R.
[patch] Honor the vxworks options overrides on VxWorksae
Hello,
The vxworks_override_option code is general enough to apply to both regular
VxWorks and VxWorksAE configurations.
The VxWorksAE configuration files miss the triggering bits, however, with
various kinds of consequences. One example is -fPIC being accepted without
-mrtp, while it should be rejected by
vxworks_override_options()
{
...
/* PIC is only supported for RTPs. */
if (flag_pic !TARGET_VXWORKS_RTP)
error (PIC is only supported for RTPs);
The attached patch fixes this by defining VXWORKS_OVERRIDE_OPTIONS for AE as
for the regular vxworks targets.
Tested by verifying that the port still builds after the change, and
that -fPIC without -mrtp is rejected as it should.
OK to commit ?
Thanks in advance,
With Kind Regards
2014-05-30 Olivier Hainque hain...@adacore.com
* config/vxworksae.h (VXWORKS_OVERRIDE_OPTIONS): Define.
Re: [AArch64] Implement some vca*_f[32,64] intrinsics
Ping. https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01771.html Kyrill On 23/06/14 15:30, Kyrill Tkachov wrote: Hi all, This patch implements some absolute compare intrinsics in arm_neon.h. Execution tests are added. Tested aarch64-none-elf, aarch64_be-none-elf, bootstrapped on aarch64 linux Ok for trunk? 2014-06-23 Kyrylo Tkachov kyrylo.tkac...@arm.com * config/aarch64/arm_neon.h (vcage_f64): New intrinsic. (vcagt_f64): Likewise. (vcale_f64): Likewise. (vcaled_f64): Likewise. (vcales_f32): Likewise. (vcalt_f64): Likewise. (vcaltd_f64): Likewise. (vcalts_f32): Likewise. 2014-06-23 Kyrylo Tkachov kyrylo.tkac...@arm.com * gcc.target/aarch64/simd/vcage_f64.c: New test. * gcc.target/aarch64/simd/vcagt_f64.c: Likewise. * gcc.target/aarch64/simd/vcale_f64.c: Likewise. * gcc.target/aarch64/simd/vcaled_f64.c: Likewise. * gcc.target/aarch64/simd/vcales_f32.c: Likewise. * gcc.target/aarch64/simd/vcalt_f64.c: Likewise. * gcc.target/aarch64/simd/vcaltd_f64.c: Likewise. * gcc.target/aarch64/simd/vcalts_f32.c: Likewise.
Re: [PATCH][AArch64] Implement vfma_f64, vmla_f64, vfms_f64, vmls_f64 intrinsics
Ping. https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01615.html Kyrill On 20/06/14 15:17, Kyrill Tkachov wrote: Hi all, Now that Alan fixed the float64x1_t machinery, this patch implements some low-hanging intrinsics in arm_neon.h. Tested aarch64-none-elf and bootstrapped on aarch64-linux. Ok for trunk? Thanks, Kyrill 2014-06-20 Kyrylo Tkachov kyrylo.tkac...@arm.com * config/aarch64/arm_neon.h (vfma_f64): New intrinsic. (vmla_f64): Likewise. (vfms_f64): Likewise. (vmls_f64): Likewise. 2014-06-20 Kyrylo Tkachov kyrylo.tkac...@arm.com * gcc.target/aarch64/simd/vfma_f64.c: New test. * gcc.target/aarch64/simd/vmla_f64.c: Likewise. * gcc.target/aarch64/simd/vfms_f64.c: Likewise. * gcc.target/aarch64/simd/vmls_f64.c: Likewise.
Re: [patch] Honor the vxworks options overrides on VxWorksae
With the patch attached ...
On Jun 30, 2014, at 15:29 , Olivier Hainque hain...@adacore.com wrote:
Hello,
The vxworks_override_option code is general enough to apply to both regular
VxWorks and VxWorksAE configurations.
The VxWorksAE configuration files miss the triggering bits, however, with
various kinds of consequences. One example is -fPIC being accepted without
-mrtp, while it should be rejected by
vxworks_override_options()
{
...
/* PIC is only supported for RTPs. */
if (flag_pic !TARGET_VXWORKS_RTP)
error (PIC is only supported for RTPs);
The attached patch fixes this by defining VXWORKS_OVERRIDE_OPTIONS for AE as
for the regular vxworks targets.
Tested by verifying that the port still builds after the change, and
that -fPIC without -mrtp is rejected as it should.
OK to commit ?
Thanks in advance,
With Kind Regards
2014-05-30 Olivier Hainque hain...@adacore.com
* config/vxworksae.h (VXWORKS_OVERRIDE_OPTIONS): Define.
vxae-override.diff
Description: Binary data
Re: Optimize type streaming
Note that testusite passes with the patch; ... Confirmed. There is a typo s/fileds/fields/ I have seen the failures because I now run the gfortran testsuite with the addition of '-g -flto'. Is it worth pushing in this direction? Cheers, Dominique
[C++ Patch] PR 54891
Hi,
I think it's fair to say that this issue is rather tricky, considering
that among the compilers I have at hand none gets it right without
regressing on parse/pr26997.C. The basic issue is simple: in C++11
(void)[]{};
is well formed, thus cp_parser_tokens_start_cast_expression should be
changed to return non-zero when a '[' follows the parenthesized type-id.
Then, however, if we do nothing else we regress on the line:
(C ())[2];
of the above testcase, because in that case we have a '[' but in fact we
don't have a cast-expression - ie, we don't have a lambda-expression -
and we want to parse the whole expression as unary-expression. Thus I
figured out that in such ambiguous cases we can avoid committing (we
used to call cp_parser_parse_definitely which becomes a conditional
cp_parser_commit_to_topmost_tentative_parse) and instead first try
cp_parser_cast_expression and then fall back to
cp_parser_unary_expression (if cp_parser_parse_definitely returns false
after the former). Tested x86_64-linux.
Thanks,
Paolo.
/cp
2014-06-30 Paolo Carlini paolo.carl...@oracle.com
PR c++/54891
* parser.c (cp_parser_tokens_start_cast_expression): In C++11
a '[' can also start a primary-expression.
(cp_parser_cast_expression): Parse a cast-expression only tentatively
when cp_parser_tokens_start_cast_expression returns -1.
/testsuite
2014-06-30 Paolo Carlini paolo.carl...@oracle.com
PR c++/54891
* g++.dg/cpp0x/lambda/lambda-cast1.C: New.
Index: cp/parser.c
===
--- cp/parser.c (revision 212119)
+++ cp/parser.c (working copy)
@@ -4109,6 +4109,7 @@ complain_flags (bool decltype_p)
this
( expression )
id-expression
+ lambda-expression (C++11)
GNU Extensions:
@@ -7621,10 +7622,10 @@ cp_parser_delete_expression (cp_parser* parser)
tf_warning_or_error);
}
-/* Returns true if TOKEN may start a cast-expression and false
- otherwise. */
+/* Returns 1 if TOKEN may start a cast-expression and, in C++11,
+ isn't '[', -1 if the TOKEN is '[' in C++11, 0 otherwise. */
-static bool
+static int
cp_parser_tokens_start_cast_expression (cp_parser *parser)
{
cp_token *token = cp_lexer_peek_token (parser-lexer);
@@ -7667,7 +7668,7 @@ cp_parser_tokens_start_cast_expression (cp_parser
case CPP_OR:
case CPP_OR_OR:
case CPP_EOF:
- return false;
+ return 0;
case CPP_OPEN_PAREN:
/* In ((type ()) () the last () isn't a valid cast-expression,
@@ -7675,12 +7676,15 @@ cp_parser_tokens_start_cast_expression (cp_parser
return cp_lexer_peek_nth_token (parser-lexer, 2)-type
!= CPP_CLOSE_PAREN;
- /* '[' may start a primary-expression in obj-c++. */
+ /* '[' may start a primary-expression in obj-c++ and in C++11,
+as a lambda expression, eg, '(void)[]{}'. */
case CPP_OPEN_SQUARE:
+ if (cxx_dialect = cxx11)
+ return -1;
return c_dialect_objc ();
default:
- return true;
+ return 1;
}
}
@@ -7705,7 +7709,7 @@ cp_parser_cast_expression (cp_parser *parser, bool
{
tree type = NULL_TREE;
tree expr = NULL_TREE;
- bool cast_expression_p;
+ int cast_expression = 0;
const char *saved_message;
/* There's no way to know yet whether or not this is a cast.
@@ -7728,6 +7732,7 @@ cp_parser_cast_expression (cp_parser *parser, bool
will commit to the parse at that point, because we cannot
undo the action that is done when creating a new class. So,
then we cannot back up and do a postfix-expression.
+
Another tricky case is the following (c++/29234):
struct S { void operator () (); };
@@ -7746,20 +7751,30 @@ cp_parser_cast_expression (cp_parser *parser, bool
we are dealing with an unary-expression, a postfix-expression
or something else.
+Yet another tricky case, in C++11, is the following (c++/54891):
+
+(void)[]{};
+
+ The issue is that usually, besides the case of lambda-expressions,
+the parenthesized type-id cannot be followed by '[', and, eg, we
+want to parse '(C ())[2];' in parse/pr26997.C as unary-expression.
+Thus, if cp_parser_tokens_start_cast_expression returns -1, below
+we don't commit, we try a cast-expression, then an unary-expression.
+
Save tokens so that we can put them back. */
cp_lexer_save_tokens (parser-lexer);
/* We may be looking at a cast-expression. */
- cast_expression_p
- = (cp_parser_skip_to_closing_parenthesis (parser, false, false,
- /*consume_paren=*/true)
- cp_parser_tokens_start_cast_expression (parser));
+ if (cp_parser_skip_to_closing_parenthesis (parser, false, false,
+
Re: [C++ Patch] PR 54891
OK. Jason
Re: [PATCH][sched-deps] Generalise usage of macro fusion to work on any two insns
On 27/06/14 09:29, Kyrill Tkachov wrote: Hi all, This patch generalises the TARGET_MACRO_FUSION_PAIR_P hook usage to work on more than just compares and conditional branches for which it was initially designed for (for x86). There are some instructions in arm and aarch64 that can be fused together when they're back to back in the instruction stream and I'd like to use this hook to keep them together. I'll post an implementation of TARGET_MACRO_FUSION_PAIR_P for arm and aarch64 shortly... Bootstrapped and tested on x86, aarch64-none-linux-gnu and arm-none-linux-gnueabihf. Ok for trunk? 2014-06-27 Ramana Radhakrishnan ramana.radhakrish...@arm.com Kyrylo Tkachov kyrylo.tkac...@arm.com * sched-deps.c (try_group_insn): Generalise macro fusion hook usage to any two insns. Update comment. Hmm, found a bug in this patch. Will respin... Kyrill
Re: [patch libstdc++] Add xmethods for std::vector and std::unique_ptr
Siva == Siva Chandra sivachan...@google.com writes:
Siva +# Load the xmethods.
Siva +from libstdcxx.v6.xmethods import register_libstdcxx_xmethods
Siva +register_libstdcxx_xmethods (gdb.current_objfile ())
I don't think any addition to the hook file should be needed.
Siva +# The object to be returned is the 0-th element in the tuple
_m_t.
Siva +# The following retrieves the 0-th element of this tuple.
Siva +_m_t_base = _m_t[_m_t.type.fields()[0]] # std::tuple has a
single base
Siva +# class and no data
members.
Siva +for f in _m_t_base.type.fields():
Siva +# The object is embedded in the _Head_base base class of
Siva +# _m_t_base.
Siva +if f.is_base_class and f.name.find('std::_Head_base') == 0:
Siva +_head_base = _m_t_base[f]
Did you investigate sharing any code with the printers?
If so, why did you choose not to?
If not, could you please look into that?
Tom
C++ PATCH for c++/61539 (type/value mismatch in variadic deduction)
unify_one_argument was assuming that there's no way we can get a
type/value mismatch in a nested deduction, but it can happen with
variadic templates; deduction should just fail in that case.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 40c2934a737ba5d2e406e079a2b249245dfb17ab
Author: Jason Merrill ja...@redhat.com
Date: Mon Jun 30 10:41:22 2014 -0400
PR c++/61539
* pt.c (unify_one_argument): Type/expression mismatch just causes
deduction failure.
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f0a598b..7f33b6d 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -16501,8 +16501,9 @@ unify_one_argument (tree tparms, tree targs, tree parm, tree arg,
maybe_adjust_types_for_deduction (strict, parm, arg, arg_expr);
}
else
-gcc_assert ((TYPE_P (parm) || TREE_CODE (parm) == TEMPLATE_DECL)
- == (TYPE_P (arg) || TREE_CODE (arg) == TEMPLATE_DECL));
+if ((TYPE_P (parm) || TREE_CODE (parm) == TEMPLATE_DECL)
+ != (TYPE_P (arg) || TREE_CODE (arg) == TEMPLATE_DECL))
+ return unify_template_argument_mismatch (explain_p, parm, arg);
/* For deduction from an init-list we need the actual list. */
if (arg_expr BRACE_ENCLOSED_INITIALIZER_P (arg_expr))
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic160.C b/gcc/testsuite/g++.dg/cpp0x/variadic160.C
new file mode 100644
index 000..20fcd5b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic160.C
@@ -0,0 +1,49 @@
+// PR c++/61539
+// { dg-do compile { target c++11 } }
+
+template typename _CharT class A;
+template typename class B;
+template class charT class C;
+template class Cchar
+{
+ virtual void xparse (int , const BAchar ) const;
+};
+template class T, class charT = char class G : CcharT
+{
+public:
+ G (void *) {}
+ void default_value (const T );
+ void xparse (int , const BAcharT ) const;
+};
+template class T, class charT
+void validate (int , const BAcharT , T *, int);
+template class T, class charT
+void GT, charT::xparse (int p1, const BAcharT p2) const
+{
+ validate (p1, p2, (T *)0, 0);
+}
+template class T GT *value (T *) { return new GT(0); }
+namespace Eigen
+{
+template typename T struct D;
+template typename, int, int, int = 0, int = 0, int = 0 class F;
+template typename _Scalar, int _Rows, int _Cols, int _Options, int _MaxRows,
+ int _MaxCols
+struct DF_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols
+{
+ typedef _Scalar Scalar;
+};
+template typename, int, int, int, int, int _MaxCols class F
+{
+public:
+ typedef typename Eigen::DF::Scalar Scalar;
+ F (const Scalar , const Scalar , const Scalar );
+};
+template class... T
+void validate (int , const BAchar , Eigen::FT... *);
+}
+int main (int, char *[])
+{
+ Eigen::Fdouble, 3, 1 a (0, 0, 0);
+ value (a)-default_value (Eigen::Fdouble, 3, 1(0, 0, 0));
+}
Re: Regimplification enhancements 3/3
On 06/17/2014 04:54 PM, Martin Jambor wrote:
Weird... does the following (untested) patch help?
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 0afa197..747b1b6 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -3277,6 +3277,8 @@ sra_modify_assign (gimple *stmt, gimple_stmt_iterator
*gsi)
if (modify_this_stmt
|| gimple_has_volatile_ops (*stmt)
+ || is_gimple_reg (lhs)
+ || is_gimple_reg (rhs)
|| contains_vce_or_bfcref_p (rhs)
|| contains_vce_or_bfcref_p (lhs)
|| stmt_ends_bb_p (*stmt))
Unfortunately not.
It is just a quick thought though. If it does not, could you post the
access trees dumped by -fdump-tree-esra-details or
-fdump-tree-sra-details (depending on whether this is early or late
SRA)? Or is it simple to set it up locally?
Not really. It needs a whole patch tree for the ptx port. I'm attaching
the last two dump files.
Bernd
;; Function bar (bar, funcdef_no=0, decl_uid=1376, symbol_order=0)
Pass statistics:
Pass statistics:
bar (struct S xD.1375)
{
struct S D.1385;
struct S aD.1378;
struct S D.1379;
struct S D.1381;
;; basic block 2, loop depth 0, count 0, freq 1, maybe hot
;;prev block 0, next block 1, flags: (NEW, REACHABLE)
;;pred: ENTRY [100.0%] (FALLTHRU,EXECUTABLE)
# .MEM_2 = VDEF .MEM_1(D)
aD.1378 = xD.1375;
# .MEM_3 = VDEF .MEM_2
# USE = nonlocal
# CLB = nonlocal
_6 = fooD.1374 (aD.1378);
# .MEM_7 = VDEF .MEM_3
D.1379 = _6;
# .MEM_4 = VDEF .MEM_7
aD.1378 ={v} {CLOBBER};
# .MEM_5 = VDEF .MEM_4
D.1381 = D.1379;
# VUSE .MEM_5
return D.1381;
;;succ: EXIT [100.0%]
}
;; Function bar (bar, funcdef_no=0, decl_uid=1376, symbol_order=0)
Pass statistics:
Candidate (1375): x
Candidate (1385): D.1385
Candidate (1378): a
Candidate (1379): D.1379
Candidate (1381): D.1381
Will attempt to totally scalarize D.1379 (UID: 1379):
! Disqualifying D.1385 - No or inhibitingly overlapping accesses.
! Disqualifying x - No scalar replacements to be created.
! Disqualifying a - No scalar replacements to be created.
Created a replacement for D.1379 offset: 0, size: 32: SR$2
Access trees for D.1379 (UID: 1379):
access { base = (1379)'D.1379', offset = 0, size = 32, expr = D.1379.len, type
= unsigned int, grp_read = 1, grp_write = 1, grp_assignment_read = 1,
grp_assignment_write = 1, grp_scalar_read = 1, grp_scalar_write = 0,
grp_total_scalarization = 1, grp_hint = 1, grp_covered = 1,
grp_unscalarizable_region = 0, grp_unscalarized_data = 0, grp_partial_lhs = 0,
grp_to_be_replaced = 1, grp_to_be_debug_replaced = 0, grp_maybe_modified = 0,
grp_not_necessarilly_dereferenced = 0
! Disqualifying D.1381 - No scalar replacements to be created.
Pass statistics:
Scalarized aggregates: 1
Modified expressions: 2
Separate LHS and RHS handling: 2
Scalar replacements created: 1
Updating SSA:
Registering new PHI nodes in block #0
Registering new PHI nodes in block #2
Updating SSA information for statement SR$2 = MEM[(struct S *)_6];
Updating SSA information for statement MEM[(struct S *)D.1381] = SR$2;
DFA Statistics for bar
-
Number ofMemory
instances used
-
USE operands 1 8b
DEF operands 2 16b
VUSE operands 6 48b
VDEF operands 4 32b
PHI nodes 0 0b
PHI arguments 0 0b
-
Total memory used by DFA/SSA data 104b
-
Hash table statistics:
var_infos: size 61, 1 elements, 0.00 collision/search ratio
Symbols to be put in SSA form
{ D.1387 }
Incremental SSA update started at block: 0
Number of blocks in CFG: 3
Number of blocks to update: 2 ( 67%)
Affected blocks: 0 2
Re: [C++ Patch] PR 51400
OK. Jason
Re: [PATCH PR C++/58781, 59867, 60249 ] Various user-defined string literal issues involving character encodings, dropped bytes, semi-infinite loops
On 06/28/2014 05:57 PM, Ed Smith-Rowland wrote: OK? I would also like to apply this to 4.9. OK. Jason
[PING][PATCH] Fix for PR 61561
This is a reminder message on fix for PR 61561 which is ICE while
compiling something which is valid* 'C' code.
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61561).
Ramana, Richard, I am very appreciated for your attention and error
indication, your work made my patch simple and tiny. And I hope that
this patch will save somebody's time.
Additional info about the issue:
By adding constrain 'k' we want to make gcc work properly with stack
register in HI and QI mode. This is need because CSE an RTL phase in
some cases propagates sp register directly to HI/QI memory set. Next
reload an RTL phase trying to build HI/QI move sp into some general
purpose register which is fail due to pattern absence. You may ask who
needs part of sp register? Nevertheless it is legitimate operation and
there is simple example of practical usage. Imagine you put some local
variable 'a' into hash table which has 256 bunches. In this case
'operator []' of some hash class may take QI from sp register which is
directly pointing on 'a'.
Patch was reg. tested on --build=x86_64-pc-linux-gnu
--host=x86_64-pc-linux-gnu --target=arm-v7a15v5r2-linux-gnueabi for
c,c++,fortran languages w/o bootstrap.
* According to 'C' Standard [ISO/IEC 9899:2011] 6.3.2.3
Any pointer type may be converted to an integer type. Except as
previously specified, the result is implementation-defined. As we know
'char' and 'short' (also called 'short int') are both integers.
Thank for your attention.
Marat.
gcc/ChangeLog:
2014-06-30 Marat Zakirov m.zaki...@samsung.com
PR target/61561
* config/arm/arm.md (*movhi_insn_arch4): Handle stack pointer.
(*movhi_bytes): Likewise.
(*arm_movqi_insn): Likewise.
gcc/testsuite/ChangeLog:
2014-06-30 Marat Zakirov m.zaki...@samsung.com
PR target/61561
* gcc.dg/pr61561.c: New test.
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 42c12c8..99290dc 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -6291,7 +6291,7 @@
;; Pattern to recognize insn generated default case above
(define_insn *movhi_insn_arch4
[(set (match_operand:HI 0 nonimmediate_operand =r,r,m,r)
- (match_operand:HI 1 general_operand rI,K,r,mi))]
+ (match_operand:HI 1 general_operand rIk,K,r,mi))]
TARGET_ARM
arm_arch4
(register_operand (operands[0], HImode)
@@ -6315,7 +6315,7 @@
(define_insn *movhi_bytes
[(set (match_operand:HI 0 s_register_operand =r,r,r)
- (match_operand:HI 1 arm_rhs_operand I,r,K))]
+ (match_operand:HI 1 arm_rhs_operand I,rk,K))]
TARGET_ARM
@
mov%?\\t%0, %1\\t%@ movhi
@@ -6430,7 +6430,7 @@
(define_insn *arm_movqi_insn
[(set (match_operand:QI 0 nonimmediate_operand =r,r,r,l,r,l,Uu,r,m)
- (match_operand:QI 1 general_operand r,r,I,Py,K,Uu,l,m,r))]
+ (match_operand:QI 1 general_operand rk,rk,I,Py,K,Uu,l,m,r))]
TARGET_32BIT
( register_operand (operands[0], QImode)
|| register_operand (operands[1], QImode))
diff --git a/gcc/testsuite/gcc.dg/pr61561.c b/gcc/testsuite/gcc.dg/pr61561.c
new file mode 100644
index 000..0f4b716
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr61561.c
@@ -0,0 +1,15 @@
+/* PR c/61561. */
+/* { dg-do assemble } */
+/* { dg-options -w -O2 } */
+
+int dummy (int a);
+
+char a;
+short b;
+
+void mmm (void)
+{
+ char dyn[dummy (3)];
+ a = (char)dyn[0];
+ b = (short)dyn[0];
+}
Re: [PATCH, rs6000, testsuite] Skip gfortran.dg/round_4.f90 for PowerPC Linux
On Sun, Jun 29, 2014 at 9:48 PM, Bill Schmidt wschm...@linux.vnet.ibm.com wrote: Hi, The test in gfortran.dg/round_4.f90, introduced in GCC 4.9, checks for correct behavior of different rounding modes. However, for quad-precision floating-point, it requires that the number 0.100481 be exactly represented. Since the PowerPC long double implementation (double-double) only guarantees 31 bits of precision, the test fails for the real(qp) portions of the test. Thus this patch marks the test invalid for PowerPC Linux for now. (We may want to also do this for other subtargets; let me know if so.) At such time as IEEE 128-bit floating-point is supported by the PowerPC port, we should revisit this. Is this ok for trunk and 4.9? Thanks, Bill 2014-06-29 Bill Schmidt wschm...@linux.vnet.ibm.com * gfortran.dg/round_4.f90: Skip for powerpc*-*-linux* since the test requires greater precision than the current PowerPC long double implementation supports. Okay. I wonder if this is passing on AIX because AIX defaults to double? Thanks, David
Re: Optimize type streaming
Note that testusite passes with the patch; ... Confirmed. There is a typo s/fileds/fields/ I have seen the failures because I now run the gfortran testsuite with
Re: [PATCH v2, rtl]: Teach _.barriers and _.eh_range passes to not split a call and its corresponding CALL_ARG_LOCATION note.
On 06/29/2014 12:51 PM, Uros Bizjak wrote:
I believe that attached v2 patch addresses all your review comments.
Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Looks good, thanks.
r~
Re: [C++] Avoid producing duplicate binfos for member pointers
On 06/30/2014 08:42 AM, Jason Merrill wrote: Hmm, I wonder why we need BINFOs at all for the ptrmemfunc RECORD_TYPEs, which are not classes. I'm working on removing both BINFO and TYPE_LANG_SPECIFIC. Jason
Re: [PATCH, alpha]: Wrap {un,}aligned_store sequence with memory blockages.
On 06/29/2014 11:14 AM, Uros Bizjak wrote: if (MEM_READONLY_P (x)) +if (GET_CODE (mem_addr) == AND) + return 1; return 0; Certainly missing braces here. But with that fixed the patch looks plausible. I'll look at it closer later today. r~
Re: [PATCH, rs6000, testsuite] Skip gfortran.dg/round_4.f90 for PowerPC Linux
On Jun 29, 2014, at 6:48 PM, Bill Schmidt wschm...@linux.vnet.ibm.com wrote:
+! { dg-skip-if IBM long double 31 bits of precision, test requires 38 {
powerpc*-*-linux* } }
Thanks for the comment. Can someone confirm that this fails on darwin and
should be skipped there as well?
Re: [C++] Avoid producing duplicate binfos for member pointers
On 06/30/2014 08:42 AM, Jason Merrill wrote: Hmm, I wonder why we need BINFOs at all for the ptrmemfunc RECORD_TYPEs, which are not classes. I'm working on removing both BINFO and TYPE_LANG_SPECIFIC. Works for me! :) Thanks, Honza
[C PATCH] Add -Wincompatible-pointer-types option (PR c/58286)
This patch adds the -Wincompatible-pointer-types option for a warning
we already have, so it's possible to suppress this specific warning, or
use it with -Werror= and so on.
As a followup change, I'm considering printing the types of the pointers;
saying merely e.g. assignment from incompatible pointer type seems to be too
austere. (We say expected T but argument is of type U when passing
arguments.)
This is for C/ObjC only, since in C++, we'd issue cannot convert error.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2014-06-30 Marek Polacek pola...@redhat.com
PR c/58286
* doc/invoke.texi: Document -Wincompatible-pointer-types.
c-family/
* c.opt (Wincompatible-pointer-types): New option.
c/
* c-typeck.c (convert_for_assignment): Pass
OPT_Wincompatible_pointer_types instead of 0 to WARN_FOR_ASSIGNMENT.
testsuite/
* gcc.dg/Wincompatible-pointer-types.c: New test.
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 1d02bae..6448b1b 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -443,6 +443,10 @@ Wignored-qualifiers
C C++ Var(warn_ignored_qualifiers) Warning EnabledBy(Wextra)
Warn whenever type qualifiers are ignored.
+Wincompatible-pointer-types
+C ObjC Var(warn_incompatible_pointer_types) Init(1) Warning
+Warn when there is a conversion between pointers that have incompatible types
+
Winit-self
C ObjC C++ ObjC++ Var(warn_init_self) Warning LangEnabledBy(C++ ObjC++,Wall)
Warn about variables which are initialized to themselves
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index b62e830..fff26a3 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -6189,7 +6189,8 @@ convert_for_assignment (location_t location, location_t
expr_loc, tree type,
else
/* Avoid warning about the volatile ObjC EH puts on decls. */
if (!objc_ok)
- WARN_FOR_ASSIGNMENT (location, expr_loc, 0,
+ WARN_FOR_ASSIGNMENT (location, expr_loc,
+ OPT_Wincompatible_pointer_types,
G_(passing argument %d of %qE from
incompatible pointer type),
G_(assignment from incompatible pointer type),
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index dbc1132..dfae4f0 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -251,7 +251,7 @@ Objective-C and Objective-C++ Dialects}.
-Wno-format-contains-nul -Wno-format-extra-args -Wformat-nonliteral @gol
-Wformat-security -Wformat-signedness -Wformat-y2k @gol
-Wframe-larger-than=@var{len} -Wno-free-nonheap-object -Wjump-misses-init @gol
--Wignored-qualifiers @gol
+-Wignored-qualifiers -Wincompatible-pointer-types @gol
-Wimplicit -Wimplicit-function-declaration -Wimplicit-int @gol
-Winit-self -Winline @gol
-Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
@@ -4199,14 +4199,19 @@ This option is only active when @option{-ftree-vrp} is
active
(default for @option{-O2} and above). It warns about subscripts to arrays
that are always out of bounds. This warning is enabled by @option{-Wall}.
-@item -Wno-discarded-qualifiers
+@item -Wno-discarded-qualifiers @r{(C and Objective-C only)}
@opindex Wno-discarded-qualifiers
@opindex Wdiscarded-qualifiers
Do not warn if type qualifiers on pointers are being discarded.
Typically, the compiler will warn if a @code{const char *} variable is
passed to a function that takes @code{char *} parameter. This option
-can be used to suppress such a warning. This warning is only supported
-for C.
+can be used to suppress such a warning.
+
+@item -Wno-incompatible-pointer-types @r{(C and Objective-C only)}
+@opindex Wno-incompatible-pointer-types
+@opindex Wincompatible-pointer-types
+Do not warn when there is a conversion between pointers that have incompatible
+types.
@item -Wno-div-by-zero
@opindex Wno-div-by-zero
diff --git gcc/testsuite/gcc.dg/Wincompatible-pointer-types.c
gcc/testsuite/gcc.dg/Wincompatible-pointer-types.c
index e69de29..e765b27 100644
--- gcc/testsuite/gcc.dg/Wincompatible-pointer-types.c
+++ gcc/testsuite/gcc.dg/Wincompatible-pointer-types.c
@@ -0,0 +1,21 @@
+/* PR c/58286 */
+/* { dg-do compile } */
+/* { dg-options -Wno-incompatible-pointer-types } */
+
+void
+fn2 (short *s, long *l)
+{
+}
+
+unsigned *
+fn1 (void)
+{
+ int (*fpi) (int);
+ int (*fpd) (double) = fpi;
+ fpi = fpd;
+ char *di;
+ float *dp = di;
+ di = dp;
+ fn2 (dp, di);
+ return dp;
+}
Marek
[C PATCH] Add -Wint-conversion option
Basically everything I wrote in the patch for -Wincompatible-pointer-types
applies here as well. A new option, -Wint-conversion (to be compatible
with clang), is added to allow more fine-grained control over the warnings.
I think we should print the types here as well, and moreover, we could hint the
user that or * may be used to fix the code.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2014-06-30 Marek Polacek pola...@redhat.com
* doc/invoke.texi: Document -Wint-conversion.
c-family/
* c.opt (Wint-conversion): New option.
c/
* c-typeck.c (convert_for_assignment): Pass OPT_Wint_conversion
instead of 0 to WARN_FOR_ASSIGNMENT.
testsuite/
* gcc.dg/Wint-conversion.c: New test.
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 6448b1b..c89040a 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -474,6 +474,10 @@ Winherited-variadic-ctor
C++ ObjC++ Var(warn_inh_var_ctor) Init(1) Warning
Warn about C++11 inheriting constructors when the base has a variadic
constructor
+Wint-conversion
+C ObjC Var(warn_int_conversion) Init(1) Warning
+Warn about incompatible integer to pointer and pointer to integer conversions
+
Wint-to-pointer-cast
C ObjC C++ ObjC++ Var(warn_int_to_pointer_cast) Init(1) Warning
Warn when there is a cast to a pointer from an integer of a different size
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index fff26a3..35bfd14 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -6213,7 +6213,8 @@ convert_for_assignment (location_t location, location_t
expr_loc, tree type,
or one that results from arithmetic, even including
a cast to integer type. */
if (!null_pointer_constant)
- WARN_FOR_ASSIGNMENT (location, expr_loc, 0,
+ WARN_FOR_ASSIGNMENT (location, expr_loc,
+OPT_Wint_conversion,
G_(passing argument %d of %qE makes
pointer from integer without a cast),
G_(assignment makes pointer from integer
@@ -6227,7 +6228,8 @@ convert_for_assignment (location_t location, location_t
expr_loc, tree type,
}
else if (codel == INTEGER_TYPE coder == POINTER_TYPE)
{
- WARN_FOR_ASSIGNMENT (location, expr_loc, 0,
+ WARN_FOR_ASSIGNMENT (location, expr_loc,
+ OPT_Wint_conversion,
G_(passing argument %d of %qE makes integer
from pointer without a cast),
G_(assignment makes integer from pointer
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index dfae4f0..e6e71c0 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -253,7 +253,7 @@ Objective-C and Objective-C++ Dialects}.
-Wframe-larger-than=@var{len} -Wno-free-nonheap-object -Wjump-misses-init @gol
-Wignored-qualifiers -Wincompatible-pointer-types @gol
-Wimplicit -Wimplicit-function-declaration -Wimplicit-int @gol
--Winit-self -Winline @gol
+-Winit-self -Winline -Wno-int-conversion @gol
-Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
-Winvalid-pch -Wlarger-than=@var{len} -Wunsafe-loop-optimizations @gol
-Wlogical-op -Wlogical-not-parentheses -Wlong-long @gol
@@ -4213,6 +4213,12 @@ can be used to suppress such a warning.
Do not warn when there is a conversion between pointers that have incompatible
types.
+@item -Wno-int-conversion @r{(C and Objective-C only)}
+@opindex Wno-int-conversion
+@opindex Wint-conversion
+Do not warn about incompatible integer to pointer and pointer to integer
+conversions.
+
@item -Wno-div-by-zero
@opindex Wno-div-by-zero
@opindex Wdiv-by-zero
diff --git gcc/testsuite/gcc.dg/Wint-conversion.c
gcc/testsuite/gcc.dg/Wint-conversion.c
index e69de29..1b7a03e 100644
--- gcc/testsuite/gcc.dg/Wint-conversion.c
+++ gcc/testsuite/gcc.dg/Wint-conversion.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options -Wno-int-conversion } */
+
+int fn1 (int *), *fn2 (int);
+
+int
+fn1 (int *p)
+{
+ int i = p;
+ i = p;
+ fn2 (p);
+ return p;
+}
+
+int *
+fn2 (int i)
+{
+ int *p = i;
+ p = i;
+ fn1 (i);
+ return i;
+}
Marek
C++ PATCH for c++/61647 (ICE with call to conversion op)
We were missing a way the function being called can be dependent.
Tested x86_64-pc-linux-gnu, applying to 4.8, 4.9, trunk.
commit 687aa57720473c7962c9eb9cf79c67cf068ba005
Author: Jason Merrill ja...@redhat.com
Date: Mon Jun 30 13:50:38 2014 -0400
PR c++/61647
* pt.c (type_dependent_expression_p): Check BASELINK_OPTYPE.
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 3ddd4b8..a188892 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -20011,7 +20011,12 @@ type_dependent_expression_p (tree expression)
return true;
if (BASELINK_P (expression))
- expression = BASELINK_FUNCTIONS (expression);
+ {
+ if (BASELINK_OPTYPE (expression)
+ dependent_type_p (BASELINK_OPTYPE (expression)))
+ return true;
+ expression = BASELINK_FUNCTIONS (expression);
+ }
if (TREE_CODE (expression) == TEMPLATE_ID_EXPR)
{
diff --git a/gcc/testsuite/g++.dg/template/conv14.C b/gcc/testsuite/g++.dg/template/conv14.C
new file mode 100644
index 000..509ae6a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/conv14.C
@@ -0,0 +1,30 @@
+// PR c++/61647
+
+class XX;
+
+templatetypename Container, typename Key
+struct Accessor;
+
+templatetypename Container, typename Key, typename KeyStore = Key
+class Variant {
+protected:
+KeyStore index;
+Container state;
+public:
+Variant(Container st, const Key i) : index(i), state(st) {}
+
+templatetypename T
+operator T() const {
+return AccessorContainer, KeyStore::template getT(state, index);
+}
+};
+
+class AutoCleanVariant : public VariantXX*, int {
+public:
+AutoCleanVariant(XX* st, int i) : VariantXX*,int(st,i) {}
+
+templatetypename T
+operator T() const {
+ return VariantXX*, int::operator T();
+}
+};
Re: [PING*3][PATCH RTL] Extend mode-switching to support toggle
On 06/30/14 06:14, Christian Bruel wrote: Hello, I still miss an approval for the middle-end part of http://gcc.gnu.org/ml/gcc-patches/2014-06/msg01038.html Didn't I ack that patch last week? https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01401.html Thanks, Jeff
C++ PATCH for c++/61566 (ICE with lambda in template default arg)
decl_mangling_context was failing to recognize a lambda in template
context as a lambda.
Tested x86_64-pc-linux-gnu, applying to 4.9 and trunk.
commit 1ba7cf95c0b024c7a3372e0734d7bf620f31dd14
Author: Jason Merrill ja...@redhat.com
Date: Mon Jun 30 12:08:09 2014 -0400
PR c++/61566
* mangle.c (decl_mangling_context): Look through a TEMPLATE_DECL.
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index ac1c1da..02c05d4 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -742,6 +742,10 @@ decl_mangling_context (tree decl)
if (tcontext != NULL_TREE)
return tcontext;
+ if (TREE_CODE (decl) == TEMPLATE_DECL
+ DECL_TEMPLATE_RESULT (decl))
+decl = DECL_TEMPLATE_RESULT (decl);
+
if (TREE_CODE (decl) == TYPE_DECL
LAMBDA_TYPE_P (TREE_TYPE (decl)))
{
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-template13.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-template13.C
new file mode 100644
index 000..adbb4db
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-template13.C
@@ -0,0 +1,20 @@
+// PR c++/61566
+// { dg-do compile { target c++11 } }
+
+struct function
+{
+ template typename _Functor
+ function (_Functor);
+};
+
+struct C
+{
+ template typename T
+ void foo (T, function = [] {});
+};
+
+void bar ()
+{
+ C c;
+ c.foo (1);
+}
Re: [C++] Avoid producing duplicate binfos for member pointers
On 06/30/2014 11:58 AM, Jason Merrill wrote:
On 06/30/2014 08:42 AM, Jason Merrill wrote:
Hmm, I wonder why we need BINFOs at all for the ptrmemfunc RECORD_TYPEs,
which are not classes.
I'm working on removing both BINFO and TYPE_LANG_SPECIFIC.
Like so. Applying to trunk.
commit 8988eb91d4be24fb630f4d8538110a5f83d44daa
Author: Jason Merrill ja...@redhat.com
Date: Mon Jun 30 08:34:33 2014 -0400
* decl.c (build_ptrmemfunc_type): Don't give a PMF RECORD_TYPE
TYPE_BINFO or TYPE_LANG_SPECIFIC.
* cp-tree.h (TYPE_PTRMEMFUNC_FLAG): Use TYPE_LANG_FLAG_2.
(TYPE_PTRMEMFUNC_P): Don't expect TYPE_LANG_SPECIFIC.
* typeck.c (build_ptrmemfunc_access_expr): Don't use lookup_member.
* pt.c (unify): Also check whether the argument is a PMF.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index c1bd7cf..1e9e1af 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -126,6 +126,7 @@ c-common.h, not after.
0: TYPE_DEPENDENT_P
1: TYPE_HAS_USER_CONSTRUCTOR.
2: TYPE_HAS_LATE_RETURN_TYPE (in FUNCTION_TYPE, METHOD_TYPE)
+ TYPE_PTRMEMFUNC_FLAG (in RECORD_TYPE)
3: TYPE_FOR_JAVA.
4: TYPE_HAS_NONTRIVIAL_DESTRUCTOR
5: CLASS_TYPE_P (in RECORD_TYPE and UNION_TYPE)
@@ -3561,11 +3562,10 @@ more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter)
function type. */
#define TYPE_PTRMEMFUNC_P(NODE) \
(TREE_CODE (NODE) == RECORD_TYPE \
-TYPE_LANG_SPECIFIC (NODE) \
TYPE_PTRMEMFUNC_FLAG (NODE))
#define TYPE_PTRMEMFUNC_FLAG(NODE) \
- (LANG_TYPE_CLASS_CHECK (NODE)-ptrmemfunc_flag)
+ (TYPE_LANG_FLAG_2 (RECORD_TYPE_CHECK (NODE)))
/* Returns true if NODE is a pointer-to-member. */
#define TYPE_PTRMEM_P(NODE) \
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 6902bb0..909f762 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -8073,13 +8073,10 @@ build_ptrmemfunc_type (tree type)
unqualified_variant
= build_ptrmemfunc_type (TYPE_MAIN_VARIANT (type));
- t = make_class_type (RECORD_TYPE);
- xref_basetypes (t, NULL_TREE);
+ t = make_node (RECORD_TYPE);
- /* Let the front end know this is a pointer to member function... */
+ /* Let the front end know this is a pointer to member function. */
TYPE_PTRMEMFUNC_FLAG (t) = 1;
- /* ... and not really a class type. */
- SET_CLASS_TYPE_P (t, 0);
field = build_decl (input_location, FIELD_DECL, pfn_identifier, type);
fields = field;
@@ -8109,7 +8106,6 @@ build_ptrmemfunc_type (tree type)
TYPE_MAIN_VARIANT (t) = unqualified_variant;
TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (unqualified_variant);
TYPE_NEXT_VARIANT (unqualified_variant) = t;
- TREE_TYPE (TYPE_BINFO (t)) = t;
}
/* Cache this pointer-to-member type so that we can find it again
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 7f33b6d..70a946c 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -18104,6 +18104,8 @@ unify (tree tparms, tree targs, tree parm, tree arg, int strict,
TYPE_PTRMEMFUNC_FN_TYPE (arg),
strict, explain_p);
}
+ else if (TYPE_PTRMEMFUNC_P (arg))
+ return unify_type_mismatch (explain_p, parm, arg);
if (CLASSTYPE_TEMPLATE_INFO (parm))
{
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 042e600..9758dfe 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -2848,8 +2848,10 @@ build_ptrmemfunc_access_expr (tree ptrmem, tree member_name)
type. */
ptrmem_type = TREE_TYPE (ptrmem);
gcc_assert (TYPE_PTRMEMFUNC_P (ptrmem_type));
- member = lookup_member (ptrmem_type, member_name, /*protect=*/0,
- /*want_type=*/false, tf_warning_or_error);
+ for (member = TYPE_FIELDS (ptrmem_type); member;
+ member = DECL_CHAIN (member))
+if (DECL_NAME (member) == member_name)
+ break;
return build_simple_component_ref (ptrmem, member);
}
Re: [PATCH 2/5] Existing call graph infrastructure enhancement
On 06/30/14 05:49, Martin Liška wrote: On 06/17/2014 10:00 PM, Jeff Law wrote: On 06/13/14 04:26, mliska wrote: Hi, this small patch prepares remaining needed infrastructure for the new pass. Changelog: 2014-06-13 Martin Liska mli...@suse.cz Honza Hubicka hubi...@ucw.cz * ipa-utils.h (polymorphic_type_binfo_p): Function marked external instead of static. * ipa-devirt.c (polymorphic_type_binfo_p): Likewise. * ipa-prop.h (count_formal_params): Likewise. * ipa-prop.c (count_formal_params): Likewise. * ipa-utils.c (ipa_merge_profiles): Be more tolerant if we merge profiles for semantically equivalent functions. * passes.c (do_per_function): If we load body of a function during WPA, this condition should behave same. * varpool.c (ctor_for_folding): More tolerant assert for variable aliases created during WPA. Presumably we don't have any useful way to merge the cases where we have provides for SRC DST in ipa_merge_profiles or even to guess which is more useful when presented with both? Does it make sense to log this into a debugging file when we drop one? Hello, this merge function was written by Honza, what do you think Honza about this note? I think this patch is fine. If adding logging makes sense, then feel free to do so and consider that trivial change pre-approved. I made a small change to this patch, where I moved 'gsi_next_nonvirtual_phi' from the pass to gimple-iterator.h. Ready for trunk with this change? Yes. I think with the exception of patch #3/5 everything looks good. I'll try to get another pass over #3 this week. What I looked at last week was pretty good; I'm pretty confident this will be wrapped up shortly. If #1/#2 make sense to install independent of #3, go ahead. #4/#5 are obviously dependent on #3. Jeff
[PATCH] Relax check against commuting XOR and ASHIFTRT in combine.c
combine.c includes a check which prevents
(ashiftrt (xor A C2) C1)
from being commuted to
(xor (ashiftrt A C1) (ashiftrt C2 C1))
for constants C1, C2 if C2 has its sign bit set.
Specifically, this prevents (ashiftrt (not A) C1) from being commuted to
(not (ashiftrt A C1))
because the former is rewritten to (ashiftrt (xor A -1) C1) above, with a
comment /* Make this fit the case below. */ - which it no longer does.
If result_mode == shift_mode, I can see no reason to prevent this normalisation
(indeed, I'm not sure I can see any reason to prevent it even if result_mode !=
shift_mode - but I've not managed to produce such a case in my own testing, as
there are always intervening subreg's or sign_extend's, or to build a toolchain
on which to reproduce the original bug, so I'm being cautious). Hence this patch
allows commutation if the two modes are equal.
As an illustrative example, on AArch64, without this patch, compiling this
snippet of the current arm_neon.h:
typedef long long int int64x1_t __attribute__ ((__vector_size__((8;
int64x1 vcgez_s64(int64x1_t a)
{
return (int64x1_t) {a =0 ? -1 : 0};
}
produces a sequence involving a logical not (mvn) followed by asr (plus some
inter-bank moves), as the combiner takes (ashiftrt (not x) 63), simplifies,
and fails to match the resulting RTL
(set (reg:DI 78 [ D.2593 ])
(ashiftrt:DI (xor:DI (reg:DI 0 x0 [ aD.2565 ])
(const_int -1 [0x]))
(const_int 63 [0x3f])))
However, with this patch, the combiner simplifies to
(set (reg:DI 84 [ D.2604 ])
(neg:DI (ge:DI (reg:DI 32 v0 [ aD.2569 ])
(const_int 0 [0]
which matches a pattern to output the desired cmge instruction.
(For the curious: the check against commutation was added in January 2004,
r76965, https://gcc.gnu.org/ml/gcc-patches/2004-01/msg03406.html - however, I've
not been able to build an ADA toolchain of that era, or an Alpha/VMS toolchain,
on which to reproduce the bug; if anyone has such and is willing and able to
investigate, by all means!)
Testing:
aarch64-none-elf: check-gcc (cross-tested)
x86-none-linux-gnu: bootstrapped; check-gcc, check-ada, check-g++
arm-none-linux-gnueabi: bootstrapped
arm-none-eabi: check-gcc (cross-tested)
gcc/ChangeLog:
* combine.c (simplify_shift_const_1): Allow commuting ASHIFTRT and XOR
if same mode.
-
diff --git a/gcc/combine.c b/gcc/combine.c
index 4e7ef55..29a9fc8 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -10219,6 +10219,7 @@ simplify_shift_const_1 (enum rtx_code code, enum machine
/* We can't do this if we have (ashiftrt (xor)) and the
constant has its sign bit set in shift_mode. */
!(code == ASHIFTRT GET_CODE (varop) == XOR
+ result_mode != shift_mode
0 trunc_int_for_mode (INTVAL (XEXP (varop, 1)),
shift_mode))
(new_rtx = simplify_const_binary_operation
@@ -10239,6 +10240,7 @@ simplify_shift_const_1 (enum rtx_code code, enum machine
for some (ashiftrt (xor)). */
if (CONST_INT_P (XEXP (varop, 1))
!(code == ASHIFTRT GET_CODE (varop) == XOR
+ result_mode != shift_mode
0 trunc_int_for_mode (INTVAL (XEXP (varop, 1)),
shift_mode)))
Re: [PATCH 3/5] IPA ICF pass
On 06/26/14 12:46, Jan Hubicka wrote: So you've added this at -O2, what is the general compile-time impact? Would it make more sense to instead have it be part of -O3, particularly since ICF is rarely going to improve performance (sans icache issues). I think code size optimization not sacrifying any (or too much of) performance are generally very welcome at -O2. Compared to LLVM and Microsoft compilers we are on code bloat side at -O2. I'm not so much worried about runtime performance here, but compile-time performance. ICF would seem like a general win as we're likely going to be more icache efficient. So, just to be clear, if the compile-time impact is minimal, then I'll fully support -O2, but my worry is that it won't be minimal :( Is returning TRUE really the conservatively correct thing to do in the absence of aliasing information? Isn't that case really I don't know in which case the proper return value is FALSE? I think with -fno-strict-aliasing the set should be 0 (Richi?) and thus we can compare for equality. We probably can be on agressive side and let 0 alias set prevail the non-0. But that can be done incrementally. I'd think it should be zero in the -fno-strict-aliasing case. My concern was that in the -fno-strict-aliasing case we seems to always assume the objects are the same. Is that the safe thing to do? That hints that the comment for the function needs tweaking. It really doesn't say anything about what the return values really mean. There are few, like we can ignore weak or visibility attribute because we do produce alias with proper visibility anyway. My plan is to start removing those attributes from declarations once they are turned into suitable representation in symbol table (or for attributes like const/noreturn/pure where we have explicit decl flags). This will make our life bit easier later, too. We probably then can whitelist some attributes, but I would deal with this later. Sure, I don't mind going with the conservative approach and iterating to remove some of the limitations. Yep, there are no resonable orders on it. If function starts same in source code they ought to end up same here. Plan was to first match for exact equality and then play with smarter tricks here. Yea, that's fine too. It's conservatively correct and we may find that there just isn't much to gain by doing hte dfs walk to build indices and such for the CFG. I guess ultimately I just want someone to look at that issue and evaluate if what we're doing now is good enough or if we're missing most of the benefit because of something like bb index stability or something dumb like that. Yep, the pass has grown up to be rather long. The gimple equality testing is the main body of work, so perhaps doing this in separate file is good idea. I'd certainly like to see that happen both because of its size and because I think those bits are useful in other contexts. Overall, most of the stuff looks quite reasonable. jeff
Re: update address taken: don't drop clobbers
On 06/28/14 16:33, Marc Glisse wrote: In an earlier version of the patch, I was using get_or_create_ssa_default_def (cfun, sym); (I was reusing the same variable). This passed bootstrap+testsuite on all languages except for ada. Indeed, the compiler wanted to coalesce several SSA_NAMEs, including those new ones, in out-of-ssa, but couldn't. And that's what you need to delve deeper into. Why did it refuse to coalesce? As long as the lifetimes do not overlap, then coalescing should have worked. jeff
Re: [PATCH] Don't ICE with huge alignment (PR middle-end/60226)
On 03/04/14 09:40, Marek Polacek wrote: This should fix ICE on insane alignment. Normally, check_user_alignment detects e.g. alignment 1 32, but not 1 28. However, record_align is in bits, so it's actually 8 * (1 28) and that's greater than INT_MAX. This patch rejects such code. In the middle hunk, we should give up when an error occurs, we don't want to call finalize_type_size in that case -- we'd ICE in there. Regtested/bootstrapped on x86_64-linux, ok for trunk? 2014-03-04 Marek Polacek pola...@redhat.com PR middle-end/60226 * stor-layout.c (layout_type): Return if alignment of array elements is greater than element size. Error out if requested alignment is too large. cp/ * class.c (layout_class_type): Error out if requested alignment is too large. testsuite/ * c-c++-common/pr60226.c: New test. Is this still applicable after the wide-int changes? I haven't looked closely. jeff
[PATCH, alpha]: Introduce handle_trap_shadows and align_insns passes
Hello!
After fixing _.barriers and _.eh_range passes w.r.t. CALL_ARG_LOCATION
notes, we can finaly move handling of trap shadows (PR 56858) and insn
alignments into their own passes.
Additionally, the patch skips handling of BARRIERs in
alpha_pad_function_end, since CALL_ARG_LOCATION notes are not split
away from their call insn anymore.
2014-06-30 Uros Bizjak ubiz...@gmail.com
PR target/56858
* config/alpha/alpha.c: Include tree-pass.h, context.h
and pass_manager.h.
(pass_data_handle_trap_shadows): New pass.
(pass_handle_trap_shadows::gate): New pass gate function.
(make_pass_handle_trap_shadows): New function.
(rest_of_handle_trap_shadows): Ditto.
(alpha_align_insns_1): Rename from alpha_align_insns.
(pass_data_align_insns): New pass.
(pass_align_insns::gate): New pass gate function.
(make_pass_aling_insns): New function.
(rest_of_align_insns): Ditto.
(alpha_align_insns): Ditto.
(alpha_option_override): Declare handle_trap_shadows info
and align_insns_info. Register handle_trap_shadows and align_insns
passes here.
(alpha_reorg): Do not call alpha_trap_shadows and
alpha_align_insn from here.
(alpha_pad_function_end): Do not skip BARRIERs.
Patch was bootstrapped and regression tested on alpha-linux-gnu (the
compiler was configured with --host=alpha-linux-gnu
--build=alpha-linux-gnu --target=alpha-linux-gnu that made these
passes effective).
OK for mainline?
Uros.
Index: config/alpha/alpha.c
===
--- config/alpha/alpha.c(revision 212168)
+++ config/alpha/alpha.c(working copy)
@@ -62,6 +62,9 @@ along with GCC; see the file COPYING3. If not see
#include gimple-expr.h
#include is-a.h
#include gimple.h
+#include tree-pass.h
+#include context.h
+#include pass_manager.h
#include gimple-iterator.h
#include gimplify.h
#include gimple-ssa.h
@@ -209,6 +212,8 @@ static struct alpha_rtx_cost_data const alpha_rtx_
/* Declarations of static functions. */
static struct machine_function *alpha_init_machine_status (void);
static rtx alpha_emit_xfloating_compare (enum rtx_code *, rtx, rtx);
+static void alpha_handle_trap_shadows (void);
+static void alpha_align_insns (void);
#if TARGET_ABI_OPEN_VMS
static void alpha_write_linkage (FILE *, const char *);
@@ -217,6 +222,115 @@ static bool vms_valid_pointer_mode (enum machine_m
#define vms_patch_builtins() gcc_unreachable()
#endif
+static unsigned int
+rest_of_handle_trap_shadows (void)
+{
+ alpha_handle_trap_shadows ();
+ return 0;
+}
+
+namespace {
+
+const pass_data pass_data_handle_trap_shadows =
+{
+ RTL_PASS,
+ trap_shadows, /* name */
+ OPTGROUP_NONE, /* optinfo_flags */
+ true,/* has_execute */
+ TV_NONE, /* tv_id */
+ 0, /* properties_required */
+ 0, /* properties_provided */
+ 0, /* properties_destroyed */
+ 0, /* todo_flags_start */
+ TODO_df_finish, /* todo_flags_finish */
+};
+
+class pass_handle_trap_shadows : public rtl_opt_pass
+{
+public:
+ pass_handle_trap_shadows(gcc::context *ctxt)
+: rtl_opt_pass(pass_data_handle_trap_shadows, ctxt)
+ {}
+
+ /* opt_pass methods: */
+ virtual bool gate (function *)
+{
+ return alpha_tp != ALPHA_TP_PROG || flag_exceptions;
+}
+
+ virtual unsigned int execute (function *)
+{
+ return rest_of_handle_trap_shadows ();
+}
+
+}; // class pass_handle_trap_shadows
+
+} // anon namespace
+
+rtl_opt_pass *
+make_pass_handle_trap_shadows (gcc::context *ctxt)
+{
+ return new pass_handle_trap_shadows (ctxt);
+}
+
+static unsigned int
+rest_of_align_insns (void)
+{
+ alpha_align_insns ();
+ return 0;
+}
+
+namespace {
+
+const pass_data pass_data_align_insns =
+{
+ RTL_PASS,
+ align_insns, /* name */
+ OPTGROUP_NONE, /* optinfo_flags */
+ true,/* has_execute */
+ TV_NONE, /* tv_id */
+ 0, /* properties_required */
+ 0, /* properties_provided */
+ 0, /* properties_destroyed */
+ 0, /* todo_flags_start */
+ TODO_df_finish, /* todo_flags_finish */
+};
+
+class pass_align_insns : public rtl_opt_pass
+{
+public:
+ pass_align_insns(gcc::context *ctxt)
+: rtl_opt_pass(pass_data_align_insns, ctxt)
+ {}
+
+ /* opt_pass methods: */
+ virtual bool gate (function *)
+{
+ /* Due to the number of extra trapb insns, don't bother fixing up
+alignment when trap precision is instruction. Moreover, we can
+only do our job when sched2 is run. */
+
Re: [PATCH] Improve -fdump-tree-all efficiency
On 06/26/14 07:42, Teresa Johnson wrote: The following patch fixes a big inefficiency when using -fdump-tree-all for large source files. I found that when using this option the compile time became unreasonably slow, and I traced this to the fact that dump_begin/dump_end are called around every function/class that are dumped via -fdump-tree-original and -fdump-class-hierarchy. I fixed this by opening the .original and .class dumps once before invoking the parser and closing them once after. For a file containing ~7000 each of functions and classes, the real time measured for the compile is: no dumping: 8s -fdump-tree-all, no patch: 10m30s -fdump-tree-all, my patch: 1m21s Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk? Thanks, Teresa 2014-06-26 Teresa Johnson tejohn...@google.com * c-family/c-common.h (get_dump_info): Declare. * c-family/c-gimplify.c (c_genericize): Use saved dump files. * c-family/c-opts.c (c_common_parse_file): Begin and end dumps once around parsing invocation. (get_dump_info): New function. * cp/class.c (dump_class_hierarchy): Use saved dump files. (dump_vtable): Ditto. (dump_vtt): Ditto. Marginally gross with the special casing of the TDI_original and TDI_class dump files. Ideally one day we'll have a better dumping API which would allow us to hold open files in a sensible way. OK for the trunk. jeff
Re: [C PATCH] Add -Wincompatible-pointer-types option (PR c/58286)
On Mon, 30 Jun 2014, Marek Polacek wrote: This patch adds the -Wincompatible-pointer-types option for a warning we already have, so it's possible to suppress this specific warning, or use it with -Werror= and so on. As a followup change, I'm considering printing the types of the pointers; saying merely e.g. assignment from incompatible pointer type seems to be too austere. (We say expected T but argument is of type U when passing arguments.) This is for C/ObjC only, since in C++, we'd issue cannot convert error. Bootstrapped/regtested on x86_64-linux, ok for trunk? OK with the documentation amended to make clear this is for the cases not covered by -Wno-pointer-sign (which in ISO C terms are just as incompatible as the cases covered by the new option). -- Joseph S. Myers jos...@codesourcery.com
Re: [C PATCH] Add -Wint-conversion option
On Mon, 30 Jun 2014, Marek Polacek wrote: Basically everything I wrote in the patch for -Wincompatible-pointer-types applies here as well. A new option, -Wint-conversion (to be compatible with clang), is added to allow more fine-grained control over the warnings. I think we should print the types here as well, and moreover, we could hint the user that or * may be used to fix the code. Bootstrapped/regtested on x86_64-linux, ok for trunk? OK with the documentation amended to make clear this is about *implicit* conversions, not the cases covered by -Wno-int-to-pointer-cast and -Wno-pointer-to-int-cast. -- Joseph S. Myers jos...@codesourcery.com
C++ PATCH for c++/61659 (undefined symbol with devirtualization)
When we're devirtualizing, we need to make sure that any virtual
functions are synthesized or instantiated even if the vtable isn't going
to be emitted. My earlier patches for 53808 handled this for
synthesized destructors, but the issue is more general.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 3019e719de4a7d5972b0057c7da6bb40b198d3ab
Author: Jason Merrill ja...@redhat.com
Date: Mon Jun 30 14:39:17 2014 -0400
PR c++/61659
PR lto/53808
gcc/cp
* decl2.c (maybe_emit_vtables): Mark all vtable entries if
devirtualizing.
* init.c (build_vtbl_address): Don't mark destructor.
* class.c (finish_struct_1): Add all classes to keyed_classes
if devirtualizing.
libstdc++-v3/
* libsupc++/cxxabi.h (class __pbase_type_info): __pointer_catch
is pure, not inline.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index d3bc71e..1c28dd6 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -6405,8 +6405,10 @@ finish_struct_1 (tree t)
determine_key_method (t);
/* If a polymorphic class has no key method, we may emit the vtable
- in every translation unit where the class definition appears. */
- if (CLASSTYPE_KEY_METHOD (t) == NULL_TREE)
+ in every translation unit where the class definition appears. If
+ we're devirtualizing, we can look into the vtable even if we
+ aren't emitting it. */
+ if (CLASSTYPE_KEY_METHOD (t) == NULL_TREE || flag_devirtualize)
keyed_classes = tree_cons (NULL_TREE, t, keyed_classes);
}
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 99ea582..98897f4 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -2009,6 +2009,11 @@ maybe_emit_vtables (tree ctype)
if (DECL_COMDAT (primary_vtbl)
CLASSTYPE_DEBUG_REQUESTED (ctype))
note_debug_info_needed (ctype);
+ if (flag_devirtualize)
+ /* Make sure virtual functions get instantiated/synthesized so that
+ they can be inlined after devirtualization even if the vtable is
+ never emitted. */
+ mark_vtable_entries (primary_vtbl);
return false;
}
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 8edf519..f8cae28 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -1155,12 +1155,6 @@ build_vtbl_address (tree binfo)
/* Figure out what vtable BINFO's vtable is based on, and mark it as
used. */
vtbl = get_vtbl_decl_for_binfo (binfo_for);
- if (tree dtor = CLASSTYPE_DESTRUCTORS (DECL_CONTEXT (vtbl)))
-if (!TREE_USED (vtbl) DECL_VIRTUAL_P (dtor) DECL_DEFAULTED_FN (dtor))
- /* Make sure the destructor gets synthesized so that it can be
- inlined after devirtualization even if the vtable is never
- emitted. */
- note_vague_linkage_fn (dtor);
TREE_USED (vtbl) = true;
/* Now compute the address to use when initializing the vptr. */
diff --git a/gcc/testsuite/g++.dg/opt/devirt5.C b/gcc/testsuite/g++.dg/opt/devirt5.C
new file mode 100644
index 000..f839cbe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/devirt5.C
@@ -0,0 +1,19 @@
+// PR c++/61659
+// { dg-options -O3 }
+// { dg-final { scan-assembler-not _ZN6parserIiE9getOptionEv } }
+
+struct generic_parser_base {
+ virtual void getOption();
+ void getExtraOptionNames() { getOption(); }
+};
+template class DataType struct parser : public generic_parser_base {
+ virtual void getOption() {}
+};
+struct PassNameParser : public parserint {
+ PassNameParser();
+};
+struct list {
+ PassNameParser Parser;
+ virtual void getExtraOptionNames() { return Parser.getExtraOptionNames(); }
+};
+list PassList;
diff --git a/libstdc++-v3/libsupc++/cxxabi.h b/libstdc++-v3/libsupc++/cxxabi.h
index ab87321..5b77aee 100644
--- a/libstdc++-v3/libsupc++/cxxabi.h
+++ b/libstdc++-v3/libsupc++/cxxabi.h
@@ -298,9 +298,9 @@ namespace __cxxabiv1
__do_catch(const std::type_info* __thr_type, void** __thr_obj,
unsigned int __outer) const;
-inline virtual bool
+virtual bool
__pointer_catch(const __pbase_type_info* __thr_type, void** __thr_obj,
- unsigned __outer) const;
+ unsigned __outer) const = 0;
};
// Type information for simple pointers.
Re: [PATCH] Improve -fdump-tree-all efficiency
On Thu, Jun 26, 2014 at 06:42:09AM -0700, Teresa Johnson wrote: 2014-06-26 Teresa Johnson tejohn...@google.com * c-family/c-common.h (get_dump_info): Declare. * c-family/c-gimplify.c (c_genericize): Use saved dump files. * c-family/c-opts.c (c_common_parse_file): Begin and end dumps once around parsing invocation. (get_dump_info): New function. * cp/class.c (dump_class_hierarchy): Use saved dump files. (dump_vtable): Ditto. (dump_vtt): Ditto. Please drop the c-family/ and cp/ prefixes before committing. Marek
Re: [GOOGLE] Report the difference between profiled and guessed or annotated branch probabilities.
I refactored the code and added comments. A bug (prematurely breaking
from a loop) was fixed during the refactoring.
(My last mail was wrongly set to HTML instead of plain text. I
apologize for that.)
2014-06-30 Yi Yang ahyan...@google.com
* auto-profile.c (get_locus_information)
(fill_invalid_locus_information, record_branch_prediction_results)
(afdo_calculate_branch_prob, afdo_annotate_cfg): Main comparison and
reporting logic.
* cfg-flags.def (PREDICTED_BY_EXPECT): Add an extra flag representing
an edge's probability is predicted by annotations.
* predict.c (combine_predictions_for_bb): Set up the extra flag on an
edge when appropriate.
* common.opt (fcheck-branch-annotation)
(fcheck-branch-annotation-threshold=): Add an extra GCC option to turn
on report
On Fri, Jun 27, 2014 at 3:20 PM, Xinliang David Li davi...@google.com wrote:
Hi Yi,
1) please add comments before new functions as documentation -- follow
the coding style guideline
2) missing documenation on the new flags (pointed out by Gerald)
3) Please refactor the check code in afdo_calculate_branch_prob into a
helper function
4) the change log is not needed for google branches, but if provided,
the format should follow the style guide (e.g, function name in () ).
David
On Fri, Jun 27, 2014 at 11:07 AM, Yi Yang ahyan...@google.com wrote:
Hi,
This patch adds an option. When the option is enabled, GCC will add a
record about it in an elf section called
.gnu.switches.text.branch.annotation for every branch.
gcc/
2014-06-27 Yi Yang ahyan...@google.com
* auto-profile.c: Main comparison and reporting logic.
* cfg-flags.def: Add an extra flag representing an edge's
probability is predicted by annotations.
* predict.c: Set up the extra flag on an edge when appropriate.
* common.opt: Add an extra GCC option to turn on this report
mechanism
diff --git gcc/auto-profile.c gcc/auto-profile.c
index 74d3d1d..d0904ed 100644
--- gcc/auto-profile.c
+++ gcc/auto-profile.c
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3. If not see
#include opts.h/* for in_fnames. */
#include tree-pass.h /* for ipa pass. */
#include cfgloop.h /* for loop_optimizer_init. */
+#include md5.h /* for hashing function names. */
#include gimple.h
#include cgraph.h
#include tree-flow.h
@@ -1367,6 +1368,148 @@ afdo_propagate (void)
}
}
+/* All information parsed from a location_t that will be stored into the ELF
+ section. */
+
+struct locus_information_t {
+ /* File name of the source file containing the branch. */
+ char filename[1024];
+ /* Line number of the branch location. */
+ unsigned lineno;
+ /* Line number of the first line of function definition. */
+ unsigned function_lineno;
+ /* Hash value calculated from function name and length, used to uniquely
+ identify a function across different source versions. */
+ char function_hash[33];
+};
+
+/* Return true iff file and lineno are available for the provided locus.
+ Fill all fields of li with information about locus. */
+
+static bool
+get_locus_information (location_t locus, locus_information_t* li) {
+ if (locus == UNKNOWN_LOCATION || !LOCATION_FILE (locus))
+return false;
+ snprintf (li-filename, 1024, %s, LOCATION_FILE (locus));
+ li-lineno = LOCATION_LINE (locus);
+
+ tree block = LOCATION_BLOCK (locus);
+ tree function_decl = current_function_decl;
+
+ if (block TREE_CODE (block) == BLOCK)
+{
+ for (block = BLOCK_SUPERCONTEXT (block);
+ block (TREE_CODE (block) == BLOCK);
+ block = BLOCK_SUPERCONTEXT (block))
+ {
+ location_t tmp_locus = BLOCK_SOURCE_LOCATION (block);
+ if (LOCATION_LOCUS (tmp_locus) == UNKNOWN_LOCATION)
+ continue;
+
+ tree decl = get_function_decl_from_block (block);
+ function_decl = decl;
+ break;
+ }
+}
+
+ if (!(function_decl TREE_CODE (function_decl) == FUNCTION_DECL))
+return false;
+ unsigned function_length = 0;
+ function *f = DECL_STRUCT_FUNCTION (function_decl);
+
+ li-function_lineno = LOCATION_LINE (DECL_SOURCE_LOCATION (function_decl));
+
+ if (f)
+{
+ function_length = LOCATION_LINE (f-function_end_locus) -
+ li-function_lineno;
+}
+
+ const char *fn_name = fndecl_name (function_decl);
+ unsigned char md5_result[16];
+
+ md5_ctx ctx;
+
+ md5_init_ctx (ctx);
+ md5_process_bytes (fn_name, strlen (fn_name), ctx);
+ md5_process_bytes (function_length, sizeof (function_length), ctx);
+ md5_finish_ctx (ctx, md5_result);
+
+ /* Convert MD5 to hexadecimal representation. */
+ for (int i = 0; i 16; ++i)
+{
+ sprintf (li-function_hash + i*2, %02x, md5_result[i]);
+}
+
+ return true;
+}
+
+/* Fill li with default information representing an unknown location. */
+
+static void
+fill_invalid_locus_information (locus_information_t* li) {
+ snprintf
Re: [PATCH][sched-deps] Generalise usage of macro fusion to work on any two insns
On 06/27/14 02:29, Kyrill Tkachov wrote:
Hi all,
This patch generalises the TARGET_MACRO_FUSION_PAIR_P hook usage to work
on more than just
compares and conditional branches for which it was initially designed
for (for x86).
There are some instructions in arm and aarch64 that can be fused
together when they're back to back in the instruction stream and I'd
like to use this hook to keep them together.
I'll post an implementation of TARGET_MACRO_FUSION_PAIR_P for arm and
aarch64 shortly...
Bootstrapped and tested on x86, aarch64-none-linux-gnu and
arm-none-linux-gnueabihf.
Ok for trunk?
2014-06-27 Ramana Radhakrishnan ramana.radhakrish...@arm.com
Kyrylo Tkachov kyrylo.tkac...@arm.com
* sched-deps.c (try_group_insn): Generalise macro fusion hook usage
to any two insns. Update comment.
Isn't this going to end up calling the x86 specific macro_fusion_pair_p
with a lot more insns than that function was previously prepared to handle?
In particular I'm concerned that the 2nd argument is going to be a
non-jumping insn a lot more often. Of particular concern is this code:
test_if = SET_SRC (pc_set (condjmp));
cond = XEXP (test_if, 0);
ccode = GET_CODE (cond);
if CONDJMP is not a JUMP_INSN, pc_set is going to return NULL and XEXP
(test_if, 0) will then fault.
I realize you bootstrapped on x86, but I suspect that whatever tuning
you need to enable to really exercise this code wasn't on.
I think you can deal with this by putting
if (!any_condjump_p (condjmp)) at the start of the x86 specific
macro_fusion_pair_p is sufficient to address this issue. It also
ensures that we don't do a lot of unnecessary work in that function.
From a general code structure standpoint, can you avoid this kind of
structure:
if (any_condjmp_p (insn))
{
...
goto succ;
}
else
{
...
goto succ
}
return
succ:
Can you structure so that you return for all the cases where you don't
want to set SCHED_GROUP_P from each arm? Or go ahead and duplicate the
SCHED_GROUP_P setting in each arm of the conditional.
jeff
Re: Fix finding reg-sets of call insn in collect_fn_hard_reg_usage
On 06/26/14 04:56, Tom de Vries wrote: On 19-06-14 18:47, Richard Henderson wrote: And I forgot to mention it might be worth while to notice simple recursion. Avoid the early exit path if caller == callee, despite the caller-save info not being valid. Richard, attached patch enables handling of self-recursive functions in the fuse-caller-save optimization, and adds a test-case. I've done an x86_64 build and ran the i386.exp testsuite. OK for trunk if full bootstrap and reg-test succeeds? Yes. Thanks, Jeff
Re: Fix var-tracking ICE with COND_EXEC
On 06/27/14 08:49, Joseph S. Myers wrote: With a 4.8-based compiler for ARM, I've observed an ICE in the var-tracking.c:add_stores handling of COND_EXEC. A large testcase from building Qt can be found at https://bugs.launchpad.net/gcc-linaro/+bug/1312931; a somewhat reduced version (for the compiler with which I observed the problem) is attached (to be built with -march=armv7-a -S -g -O2 -std=c++0x -fno-exceptions -fPIE), but the issue seems very sensitive to details of optimization and the test not really suitable for the testsuite. The assertion gcc_assert (oval != v); fails. As I understand it, this is asserting that the conditional store is not storing a value already present in the location in question. Now, it's not clear to me this should be relying on optimizations like that. In this case, the key peculiarity of the code that may have caused the lack of optimization involves the shared_null static const data member. The destination of the problem conditional store holds a value loaded from that member at some point. The source of that store was previously (conditionally, on the same condition) stored into that constant location. (Maybe in the original code this store into a const location can't actually occur, but that's the code being processed after inlining.) So if you treat that location as constant it follows that the two values are the same (and that we have undefined behavior at runtime), while if you don't then the values are clearly different (the store ends up being a conditional increment). This patch simply makes add_stores do nothing in this case of a conditional store that does nothing. Tested with no regressions for cross to arm-none-linux-gnueabi. OK to commit? 2014-06-27 Joseph Myers jos...@codesourcery.com * var-tracking.c (add_stores): Return instead of asserting if old and new values for conditional store are the same. OK for the trunk. Thanks, Jeff
Re: [GOOGLE] Report the difference between profiled and guessed or annotated branch probabilities.
You don't need extra space to store file name in locus_information_t. Use pointer instead. Dehao On Mon, Jun 30, 2014 at 1:36 PM, Yi Yang ahyan...@google.com wrote: I refactored the code and added comments. A bug (prematurely breaking from a loop) was fixed during the refactoring. (My last mail was wrongly set to HTML instead of plain text. I apologize for that.) 2014-06-30 Yi Yang ahyan...@google.com * auto-profile.c (get_locus_information) (fill_invalid_locus_information, record_branch_prediction_results) (afdo_calculate_branch_prob, afdo_annotate_cfg): Main comparison and reporting logic. * cfg-flags.def (PREDICTED_BY_EXPECT): Add an extra flag representing an edge's probability is predicted by annotations. * predict.c (combine_predictions_for_bb): Set up the extra flag on an edge when appropriate. * common.opt (fcheck-branch-annotation) (fcheck-branch-annotation-threshold=): Add an extra GCC option to turn on report On Fri, Jun 27, 2014 at 3:20 PM, Xinliang David Li davi...@google.com wrote: Hi Yi, 1) please add comments before new functions as documentation -- follow the coding style guideline 2) missing documenation on the new flags (pointed out by Gerald) 3) Please refactor the check code in afdo_calculate_branch_prob into a helper function 4) the change log is not needed for google branches, but if provided, the format should follow the style guide (e.g, function name in () ). David On Fri, Jun 27, 2014 at 11:07 AM, Yi Yang ahyan...@google.com wrote: Hi, This patch adds an option. When the option is enabled, GCC will add a record about it in an elf section called .gnu.switches.text.branch.annotation for every branch. gcc/ 2014-06-27 Yi Yang ahyan...@google.com * auto-profile.c: Main comparison and reporting logic. * cfg-flags.def: Add an extra flag representing an edge's probability is predicted by annotations. * predict.c: Set up the extra flag on an edge when appropriate. * common.opt: Add an extra GCC option to turn on this report mechanism
Re: [C PATCH] Add -Wint-conversion option
Can you please add this and the other one to gcc-4.10/changes.html? I can provide help if you need any. Gerald
Re: [C PATCH] Add -Wint-conversion option
On Mon, Jun 30, 2014 at 10:51:59PM +0200, Gerald Pfeifer wrote: Can you please add this and the other one to gcc-4.10/changes.html? I can provide help if you need any. We don't have gcc-4.10/ directory, because the version of the next release is still to be decided (hopefully at Cauldron next month). Jakub
Re: [PATCH, alpha]: Wrap {un,}aligned_store sequence with memory blockages.
On Mon, Jun 30, 2014 at 5:54 PM, Richard Henderson r...@redhat.com wrote: On 06/29/2014 11:14 AM, Uros Bizjak wrote: if (MEM_READONLY_P (x)) +if (GET_CODE (mem_addr) == AND) + return 1; return 0; Certainly missing braces here. But with that fixed the patch looks plausible. I'll look at it closer later today. Uh, yes ... this RFC patch was not thoroughly tested (its intention was to illustrate the problem and possible solution). Uros.
Re: [DOC Patch] Explicit Register Variables
On 06/30/14 02:18, David Wohlferd wrote: I don't have permissions to commit this patch, but I do have a release on file with the FSF. Problem description: The text for using Explicit Register Variables is confusing, redundant, and fails to make certain essential information clear. Some specific problems: - There is no reason to call this topic Explicit Reg Vars instead of Explicit Register Variables. Agreed. - Putting text on the Explicit Register Variables menu page (instead of the Global or Local subpages) is redundant, since any text that describes the usage of Global or Local register variables will have to be repeated on the individual subpages. OK. - Vague promises of features that may some day be available are not useful in documentation (Eventually there may be a way of asking the compiler to). Can't disagree here. I believe this lame comment has been around for 20 years or longer at this point. - Vague descriptions of things that are reported to work on certain platforms are not useful (On the SPARC, there are reports that). I'd disagree. But what's more important here is the registers that are available are a function of the ABI and for someone to attempt to use this feature, they're going to have to be intimately aware of the ABI of their target. - Describing Local Register Variables as sometimes convenient for use with the extended asm feature misses the point that this is in fact the ONLY supported use for Local Register Variables. What makes you believe that this feature is only useful for extended asms? My recollection is that for the last 20+ years this feature has served effectively as a pre-allocation of certain function scoped objects into predetermined registers. - Unambiguous statements such as The following uses are explicitly /not/ supported when describing things such as calling Basic asm discourage people from attempting to misuse the feature. Not sure I really agree with this either. There's nothing inherently wrong with someone trying to use a local register variable to try and optimize code for example. ChangeLog: 2014-06-30 David Wohlferd d...@limegreensocks.com * doc/extend.texi (Explicit Reg Vars): Rewrite and clarify. * doc/implement-c.texi (Hints implementation): Use new name. So in summary, I agree with most of what you've done. The question is how to proceed at this point. We could extract the non-controversial stuff and install it now and iterate on the stuff still in the air until it's done. Or we could iterate on the stuff still in the air and install the final update as an single commit. I don't care much either way, do you have a preference? jeff dw
Re: Warn when returning the address of a temporary (middle-end) v2
On 06/29/14 03:22, Marc Glisse wrote: After looking at PR 61597, I updated the 2 conditions to: + if ((TREE_CODE (valbase) == VAR_DECL +!is_global_var (valbase)) + || TREE_CODE (valbase) == PARM_DECL) a PARM_DECL is a local variable and returning its address is wrong, isn't it? Right. It can live in either a caller or callee allocated slot. When I first glanced at the patch my thought was why is this in the path isolation pass? But I see you want to modify the returned value to be NULL. I'll have to look at why you want to do that, but at least I understand why it's utilizing the path isolation code. jeff
Re: [GOOGLE] Report the difference between profiled and guessed or annotated branch probabilities.
Fixed.
Also, I spotted some warnings caused by me using %lfs in snprintf().
I changed these to %f and tested.
On Mon, Jun 30, 2014 at 1:49 PM, Dehao Chen de...@google.com wrote:
You don't need extra space to store file name in locus_information_t.
Use pointer instead.
Dehao
On Mon, Jun 30, 2014 at 1:36 PM, Yi Yang ahyan...@google.com wrote:
I refactored the code and added comments. A bug (prematurely breaking
from a loop) was fixed during the refactoring.
(My last mail was wrongly set to HTML instead of plain text. I
apologize for that.)
2014-06-30 Yi Yang ahyan...@google.com
* auto-profile.c (get_locus_information)
(fill_invalid_locus_information, record_branch_prediction_results)
(afdo_calculate_branch_prob, afdo_annotate_cfg): Main comparison and
reporting logic.
* cfg-flags.def (PREDICTED_BY_EXPECT): Add an extra flag representing
an edge's probability is predicted by annotations.
* predict.c (combine_predictions_for_bb): Set up the extra flag on an
edge when appropriate.
* common.opt (fcheck-branch-annotation)
(fcheck-branch-annotation-threshold=): Add an extra GCC option to turn
on report
On Fri, Jun 27, 2014 at 3:20 PM, Xinliang David Li davi...@google.com
wrote:
Hi Yi,
1) please add comments before new functions as documentation -- follow
the coding style guideline
2) missing documenation on the new flags (pointed out by Gerald)
3) Please refactor the check code in afdo_calculate_branch_prob into a
helper function
4) the change log is not needed for google branches, but if provided,
the format should follow the style guide (e.g, function name in () ).
David
On Fri, Jun 27, 2014 at 11:07 AM, Yi Yang ahyan...@google.com wrote:
Hi,
This patch adds an option. When the option is enabled, GCC will add a
record about it in an elf section called
.gnu.switches.text.branch.annotation for every branch.
gcc/
2014-06-27 Yi Yang ahyan...@google.com
* auto-profile.c: Main comparison and reporting logic.
* cfg-flags.def: Add an extra flag representing an edge's
probability is predicted by annotations.
* predict.c: Set up the extra flag on an edge when appropriate.
* common.opt: Add an extra GCC option to turn on this report
mechanism
diff --git gcc/auto-profile.c gcc/auto-profile.c
index 74d3d1d..96cad49 100644
--- gcc/auto-profile.c
+++ gcc/auto-profile.c
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3. If not see
#include opts.h/* for in_fnames. */
#include tree-pass.h /* for ipa pass. */
#include cfgloop.h /* for loop_optimizer_init. */
+#include md5.h /* for hashing function names. */
#include gimple.h
#include cgraph.h
#include tree-flow.h
@@ -1367,6 +1368,148 @@ afdo_propagate (void)
}
}
+/* All information parsed from a location_t that will be stored into the ELF
+ section. */
+
+struct locus_information_t {
+ /* File name of the source file containing the branch. */
+ const char *filename;
+ /* Line number of the branch location. */
+ unsigned lineno;
+ /* Line number of the first line of function definition. */
+ unsigned function_lineno;
+ /* Hash value calculated from function name and length, used to uniquely
+ identify a function across different source versions. */
+ char function_hash[33];
+};
+
+/* Return true iff file and lineno are available for the provided locus.
+ Fill all fields of li with information about locus. */
+
+static bool
+get_locus_information (location_t locus, locus_information_t* li) {
+ if (locus == UNKNOWN_LOCATION || !LOCATION_FILE (locus))
+return false;
+ li-filename = LOCATION_FILE (locus);
+ li-lineno = LOCATION_LINE (locus);
+
+ tree block = LOCATION_BLOCK (locus);
+ tree function_decl = current_function_decl;
+
+ if (block TREE_CODE (block) == BLOCK)
+{
+ for (block = BLOCK_SUPERCONTEXT (block);
+ block (TREE_CODE (block) == BLOCK);
+ block = BLOCK_SUPERCONTEXT (block))
+ {
+ location_t tmp_locus = BLOCK_SOURCE_LOCATION (block);
+ if (LOCATION_LOCUS (tmp_locus) == UNKNOWN_LOCATION)
+ continue;
+
+ tree decl = get_function_decl_from_block (block);
+ function_decl = decl;
+ break;
+ }
+}
+
+ if (!(function_decl TREE_CODE (function_decl) == FUNCTION_DECL))
+return false;
+ unsigned function_length = 0;
+ function *f = DECL_STRUCT_FUNCTION (function_decl);
+
+ li-function_lineno = LOCATION_LINE (DECL_SOURCE_LOCATION (function_decl));
+
+ if (f)
+{
+ function_length = LOCATION_LINE (f-function_end_locus) -
+ li-function_lineno;
+}
+
+ const char *fn_name = fndecl_name (function_decl);
+ unsigned char md5_result[16];
+
+ md5_ctx ctx;
+
+ md5_init_ctx (ctx);
+ md5_process_bytes (fn_name, strlen (fn_name), ctx);
+ md5_process_bytes (function_length, sizeof (function_length), ctx);
+
Re: [C PATCH] Add -Wint-conversion option
On Mon, 30 Jun 2014, Jakub Jelinek wrote: We don't have gcc-4.10/ directory, because the version of the next release is still to be decided (hopefully at Cauldron next month). I'm a bit worried we'll miss entries in the meantime. Can we use gcc-4.10/ for now and rename later if we go for GCC V or whatever? :-) Gerald
Re: [PATCH] Relax check against commuting XOR and ASHIFTRT in combine.c
On 06/30/14 13:05, Alan Lawrence wrote:
combine.c includes a check which prevents
(ashiftrt (xor A C2) C1)
from being commuted to
(xor (ashiftrt A C1) (ashiftrt C2 C1))
for constants C1, C2 if C2 has its sign bit set.
Specifically, this prevents (ashiftrt (not A) C1) from being commuted to
(not (ashiftrt A C1))
because the former is rewritten to (ashiftrt (xor A -1) C1) above, with
a comment /* Make this fit the case below. */ - which it no longer does.
If result_mode == shift_mode, I can see no reason to prevent this
normalisation (indeed, I'm not sure I can see any reason to prevent it
even if result_mode != shift_mode - but I've not managed to produce such
a case in my own testing, as there are always intervening subreg's or
sign_extend's, or to build a toolchain on which to reproduce the
original bug, so I'm being cautious). Hence this patch allows
commutation if the two modes are equal.
As an illustrative example, on AArch64, without this patch, compiling
this snippet of the current arm_neon.h:
typedef long long int int64x1_t __attribute__ ((__vector_size__((8;
int64x1 vcgez_s64(int64x1_t a)
{
return (int64x1_t) {a =0 ? -1 : 0};
}
produces a sequence involving a logical not (mvn) followed by asr (plus
some inter-bank moves), as the combiner takes (ashiftrt (not x) 63),
simplifies, and fails to match the resulting RTL
(set (reg:DI 78 [ D.2593 ])
(ashiftrt:DI (xor:DI (reg:DI 0 x0 [ aD.2565 ])
(const_int -1 [0x]))
(const_int 63 [0x3f])))
However, with this patch, the combiner simplifies to
(set (reg:DI 84 [ D.2604 ])
(neg:DI (ge:DI (reg:DI 32 v0 [ aD.2569 ])
(const_int 0 [0]
which matches a pattern to output the desired cmge instruction.
(For the curious: the check against commutation was added in January
2004, r76965, https://gcc.gnu.org/ml/gcc-patches/2004-01/msg03406.html -
however, I've not been able to build an ADA toolchain of that era, or an
Alpha/VMS toolchain, on which to reproduce the bug; if anyone has such
and is willing and able to investigate, by all means!)
Testing:
aarch64-none-elf: check-gcc (cross-tested)
x86-none-linux-gnu: bootstrapped; check-gcc, check-ada, check-g++
arm-none-linux-gnueabi: bootstrapped
arm-none-eabi: check-gcc (cross-tested)
gcc/ChangeLog:
* combine.c (simplify_shift_const_1): Allow commuting ASHIFTRT and XOR
if same mode.
You'll want to use your sample code from above to create a testcase.
You can either dump the RTL and search it, or scan the assembly code.
Look in gcc/testsuite/gcc.target/arm for examples.
Given the original problem which prompted Kenner to make this change was
Ada related on the Alpha, you might ping rth and/or uros to see if they
could do a quick regression of those platforms with Ada enabled.
I'm naturally hesitant to approve since this was something pretty Kenner
explicitly tried to avoid AFAICT. Kenner wasn't ever known for trying
to paper over problems -- if he checked in a change like this, much more
likely than not it was well thought out and analyzed. He also probably
knew combine.c better than anyone at that time (possibly even still true
today).
Jeff
Re: [GOOGLE] Report the difference between profiled and guessed or annotated branch probabilities.
Let's use %d to replace %f (manual conversion, let's do xx%). Dehao On Mon, Jun 30, 2014 at 2:06 PM, Yi Yang ahyan...@google.com wrote: Fixed. Also, I spotted some warnings caused by me using %lfs in snprintf(). I changed these to %f and tested. On Mon, Jun 30, 2014 at 1:49 PM, Dehao Chen de...@google.com wrote: You don't need extra space to store file name in locus_information_t. Use pointer instead. Dehao On Mon, Jun 30, 2014 at 1:36 PM, Yi Yang ahyan...@google.com wrote: I refactored the code and added comments. A bug (prematurely breaking from a loop) was fixed during the refactoring. (My last mail was wrongly set to HTML instead of plain text. I apologize for that.) 2014-06-30 Yi Yang ahyan...@google.com * auto-profile.c (get_locus_information) (fill_invalid_locus_information, record_branch_prediction_results) (afdo_calculate_branch_prob, afdo_annotate_cfg): Main comparison and reporting logic. * cfg-flags.def (PREDICTED_BY_EXPECT): Add an extra flag representing an edge's probability is predicted by annotations. * predict.c (combine_predictions_for_bb): Set up the extra flag on an edge when appropriate. * common.opt (fcheck-branch-annotation) (fcheck-branch-annotation-threshold=): Add an extra GCC option to turn on report On Fri, Jun 27, 2014 at 3:20 PM, Xinliang David Li davi...@google.com wrote: Hi Yi, 1) please add comments before new functions as documentation -- follow the coding style guideline 2) missing documenation on the new flags (pointed out by Gerald) 3) Please refactor the check code in afdo_calculate_branch_prob into a helper function 4) the change log is not needed for google branches, but if provided, the format should follow the style guide (e.g, function name in () ). David On Fri, Jun 27, 2014 at 11:07 AM, Yi Yang ahyan...@google.com wrote: Hi, This patch adds an option. When the option is enabled, GCC will add a record about it in an elf section called .gnu.switches.text.branch.annotation for every branch. gcc/ 2014-06-27 Yi Yang ahyan...@google.com * auto-profile.c: Main comparison and reporting logic. * cfg-flags.def: Add an extra flag representing an edge's probability is predicted by annotations. * predict.c: Set up the extra flag on an edge when appropriate. * common.opt: Add an extra GCC option to turn on this report mechanism
Re: [PATCH] IPA REF: alias refactoring
On 06/28/2014 08:49 AM, Jan Hubicka wrote:
Hi,
this patch enhances alias manipulation for symtab_node. Honza suggested
following changes.
Patch is pre approved, will be committed if no comments and regressions.
Bootstrapped on x86_64-pc-linux-gnu, regression tests have been running.
Thanks,
Martin
gcc/ChangeLog:
* cgraph.h (iterate_direct_aliases): New function.
(FOR_EACH_ALIAS): New macro iterates all direct aliases for a node.
* cgraph.c (cgraph_for_node_thunks_and_aliases): Usage of
FOR_EACH_ALIAS added.
(cgraph_for_node_and_aliases): Likewise.
* cgraphunit.c (assemble_thunks_and_aliases): Likewise.
* ipa-inline.c (reset_edge_caches): Likewise.
(update_caller_keys): Likewise.
* trans-mem.c (ipa_tm_execute): Likewise.
*varpool.c (varpool_analyze_node): Likewise.
(varpool_for_node_and_aliases): Likewise.
* ipa-ref.h (first_referring_alias): New function.
(last_referring_alias): Likewise.
I missed it last time around, I think first_alias/last_alias are better names.
first_alias is unused. If you added it I guess FOR_EACH_ALIAS should use it.
Hello,
I renamed these functions as you suggested and has_aliases_p
predication was also added.
Previous patch has an error in ipa_ref::remove_refence, this patch has
been regtested and the problem is removed.
We probably also can bring has_aliases_p inline and implement it using
first_referring_alias.
+ /* If deleted item is IPA_REF_ALIAS, we have to move last
+ item of IPA_REF_LIST type to the deleted position. After that
+ we replace last node with deletion slot. */
+ struct ipa_ref *last_alias = list-last_referring_alias ();
You can avoid walking to last alias when the removed item is not IPA_REF_ALIAS.
+
+ /* IPA_REF_ALIAS is always put at the beginning of the list. */
inserted?
Type fixed.
If no other comments will come, I consider the patch as preapproved.
Thanks,
Martin
gcc/ChangeLog:
* cgraph.h (iterate_direct_aliases): New function.
(FOR_EACH_ALIAS): New macro iterates all direct aliases for a node.
* cgraph.c (cgraph_for_node_thunks_and_aliases): Usage of
FOR_EACH_ALIAS added.
(cgraph_for_node_and_aliases): Likewise.
* cgraphunit.c (assemble_thunks_and_aliases): Likewise.
* ipa-inline.c (reset_edge_caches): Likewise.
(update_caller_keys): Likewise.
* trans-mem.c (ipa_tm_execute): Likewise.
*varpool.c (varpool_analyze_node): Likewise.
(varpool_for_node_and_aliases): Likewise.
* ipa-ref.h (first_alias): New function.
(last_alias): Likewise.
(has_aliases_p): Likewise.
* ipa-ref.c (ipa_ref::remove_reference): Removal function
is sensitive to IPA_REF_ALIASes.
* symtab.c (symtab_node::add_reference): Node of IPA_REF_ALIAS type
are put at the beginning of the list.
(symtab_node::iterate_direct_aliases): New function.
gcc/lto/ChangeLog:
* lto-partition.c (add_symbol_to_partition_1): Usage of
FOR_EACH_ALIAS added.
OK with these changes (or if you already comitted, just do them incrementally)
Honza
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 43428be..41dcaf9 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -2198,8 +2198,7 @@ cgraph_for_node_thunks_and_aliases (struct cgraph_node *node,
bool include_overwritable)
{
struct cgraph_edge *e;
- int i;
- struct ipa_ref *ref = NULL;
+ struct ipa_ref *ref;
if (callback (node, data))
return true;
@@ -2210,16 +2209,16 @@ cgraph_for_node_thunks_and_aliases (struct cgraph_node *node,
if (cgraph_for_node_thunks_and_aliases (e-caller, callback, data,
include_overwritable))
return true;
- for (i = 0; node-iterate_referring (i, ref); i++)
-if (ref-use == IPA_REF_ALIAS)
- {
- struct cgraph_node *alias = dyn_cast cgraph_node * (ref-referring);
- if (include_overwritable
- || cgraph_function_body_availability (alias) AVAIL_OVERWRITABLE)
- if (cgraph_for_node_thunks_and_aliases (alias, callback, data,
- include_overwritable))
- return true;
- }
+
+ FOR_EACH_ALIAS (node, ref)
+{
+ struct cgraph_node *alias = dyn_cast cgraph_node * (ref-referring);
+ if (include_overwritable
+ || cgraph_function_body_availability (alias) AVAIL_OVERWRITABLE)
+ if (cgraph_for_node_thunks_and_aliases (alias, callback, data,
+ include_overwritable))
+ return true;
+}
return false;
}
@@ -2233,21 +2232,20 @@ cgraph_for_node_and_aliases (struct cgraph_node *node,
void *data,
bool include_overwritable)
{
- int i;
- struct ipa_ref *ref = NULL;
+ struct ipa_ref *ref;
if (callback (node, data))
return true;
- for (i = 0; node-iterate_referring (i, ref); i++)
-if (ref-use == IPA_REF_ALIAS)
- {
- struct cgraph_node *alias = dyn_cast cgraph_node * (ref-referring);
- if (include_overwritable
- || cgraph_function_body_availability (alias) AVAIL_OVERWRITABLE)
- if
Re: [GOOGLE] Report the difference between profiled and guessed or annotated branch probabilities.
This is intermediate result, which is meant to be consumed by further post-processing. For this reason I'd prefer to put a number without that percentage sign. I'd just output (int)(probability*1+0.5). Does this look good for you? Or maybe change that to 100 since six digits are more than enough. I don't see a reason to intentionally drop precision though. Note that for the actual probability, the best way to store it is to store the edge count, since the probability is just edge_count/bb_count. But this causes disparity in the formats of the two probabilities. On Mon, Jun 30, 2014 at 2:12 PM, Dehao Chen de...@google.com wrote: Let's use %d to replace %f (manual conversion, let's do xx%). Dehao On Mon, Jun 30, 2014 at 2:06 PM, Yi Yang ahyan...@google.com wrote: Fixed. Also, I spotted some warnings caused by me using %lfs in snprintf(). I changed these to %f and tested. On Mon, Jun 30, 2014 at 1:49 PM, Dehao Chen de...@google.com wrote: You don't need extra space to store file name in locus_information_t. Use pointer instead. Dehao On Mon, Jun 30, 2014 at 1:36 PM, Yi Yang ahyan...@google.com wrote: I refactored the code and added comments. A bug (prematurely breaking from a loop) was fixed during the refactoring. (My last mail was wrongly set to HTML instead of plain text. I apologize for that.) 2014-06-30 Yi Yang ahyan...@google.com * auto-profile.c (get_locus_information) (fill_invalid_locus_information, record_branch_prediction_results) (afdo_calculate_branch_prob, afdo_annotate_cfg): Main comparison and reporting logic. * cfg-flags.def (PREDICTED_BY_EXPECT): Add an extra flag representing an edge's probability is predicted by annotations. * predict.c (combine_predictions_for_bb): Set up the extra flag on an edge when appropriate. * common.opt (fcheck-branch-annotation) (fcheck-branch-annotation-threshold=): Add an extra GCC option to turn on report On Fri, Jun 27, 2014 at 3:20 PM, Xinliang David Li davi...@google.com wrote: Hi Yi, 1) please add comments before new functions as documentation -- follow the coding style guideline 2) missing documenation on the new flags (pointed out by Gerald) 3) Please refactor the check code in afdo_calculate_branch_prob into a helper function 4) the change log is not needed for google branches, but if provided, the format should follow the style guide (e.g, function name in () ). David On Fri, Jun 27, 2014 at 11:07 AM, Yi Yang ahyan...@google.com wrote: Hi, This patch adds an option. When the option is enabled, GCC will add a record about it in an elf section called .gnu.switches.text.branch.annotation for every branch. gcc/ 2014-06-27 Yi Yang ahyan...@google.com * auto-profile.c: Main comparison and reporting logic. * cfg-flags.def: Add an extra flag representing an edge's probability is predicted by annotations. * predict.c: Set up the extra flag on an edge when appropriate. * common.opt: Add an extra GCC option to turn on this report mechanism
Re: Warn when returning the address of a temporary (middle-end) v2
On Mon, 30 Jun 2014, Jeff Law wrote: On 06/29/14 03:22, Marc Glisse wrote: After looking at PR 61597, I updated the 2 conditions to: + if ((TREE_CODE (valbase) == VAR_DECL +!is_global_var (valbase)) + || TREE_CODE (valbase) == PARM_DECL) a PARM_DECL is a local variable and returning its address is wrong, isn't it? Right. It can live in either a caller or callee allocated slot. The caller case scares me a bit. Is it really wrong to return the address in that case? The slot still exists after returning if the caller is responsible for it. When I first glanced at the patch my thought was why is this in the path isolation pass? A very pragmatic reason is that you and Richard asked me to after v1 of the patch... It doesn't matter that much to me where this goes. But I see you want to modify the returned value to be NULL. I'll have to look at why you want to do that, but at least I understand why it's utilizing the path isolation code. Originally I mostly wanted to avoid warning several times. Now that the warning is in a pass that runs only once, it isn't that necessary (it remains necessary to do something in the front-end to avoid warning again in the middle-end). It is still an optimization (it probably helps remove dead code), though I could replace 0 with an undefined variable. -- Marc Glisse
[wwwdocs] /projects: make relative, simplify a reference
Applied. Gerald Index: projects/cfg.html === RCS file: /cvs/gcc/wwwdocs/htdocs/projects/cfg.html,v retrieving revision 1.19 diff -u -r1.19 cfg.html --- projects/cfg.html 3 Dec 2013 01:04:42 - 1.19 +++ projects/cfg.html 30 Jun 2014 22:06:09 - @@ -318,8 +318,9 @@ pointer to its basic block. It is therefore easy to adjust basic block boundary notes when instructions are reordered./dd -dta href=http://gcc.gnu.org/news/profiledriven.html;Profile +dta href=../news/profiledriven.htmlProfile based optimization infrastructure/a/dt +dd/dd /dl pPatches that are waiting for review and integration into Index: projects/cli.html === RCS file: /cvs/gcc/wwwdocs/htdocs/projects/cli.html,v retrieving revision 1.25 diff -u -r1.25 cli.html --- projects/cli.html 28 Jun 2014 10:34:16 - 1.25 +++ projects/cli.html 30 Jun 2014 22:06:09 - @@ -143,12 +143,10 @@ are followed. /p -pThere is a small README file that explains how to build and install -gcc CLI Backend and the CLI Frontend and the CLI binutils (both Mono based and DotGnu based) . -/p - -pYou can find it in the branch at -a href=http://gcc.gnu.org/viewcvs/branches/st/README?view=markup;Build instructions/a +pThere is a small +a href=https://gcc.gnu.org/svn/gcc/branches/st/README?view=markup;README + file/a that explains how to build and install the GCC CLI back end and +front end and the CLI binutils (both Mono based and DotGnu based) . /p h2a name=internalsThe CLI back end/a/h2
Re: Optimize type streaming
On June 29, 2014 9:53:03 PM CEST, Jan Hubicka hubi...@ucw.cz wrote:
In addition of pr61644 and pr61646, this commit breaks a lot of
fortran tests with -flto -O0.
Hello,
the problem here is that we have POINTER_TYPE that points to array of
variable
length (not sure why it happens only with -O0). The ICE is introduced
by the
fact that we stream the type inside of function body then and thus it
never
goes through the lto.c's type handling.
This patch fixes the ICE by moving the fixup somewhat earlier (perhaps
it is
good idea in general). I do not believe resulting IL is correct: we do
not
compute canonical type of the pointer nor we link correctly the
variants.
Richard, we probably ought to try to make all of the canonical type
fixup
happen also for function local types which would involve moving it all
from
lto.c into generic code? :((
I am testing the patch. I think it is better to have fixup here, so OK
if
it passes?
Please revert the original patch instead which was not tested properly. I'll
get back to this after I return from vacation.
Hi,
this is patch I comitted. I hope we will be able to get back to it -
especially the
sanity checking part is useful to find bugs...
(I suppose we may want to introduce type verifier)
Honza
Revert:
* tree-streamer-out.c (pack_ts_type_common_value_fields): Stream if type
is complete.
(write_ts_type_common_tree_pointers): Do not stream fields not set for
incomplete
types; do not stream duplicated fields for variants; sanity check that
variant
and type match.
(write_ts_type_non_common_tree_pointers): Likewise.
* tree-streamer-in.c (unpack_ts_type_common_value_fields): Mark in
TYPE_SIZE whether
type is complete.
(lto_input_ts_type_common_tree_pointers): Do same changes as in
write_ts_type_common_tree_pointers
(lto_input_ts_type_non_common_tree_pointers): Likewise.
* lto.c (lto_copy_fields_not_streamed): New function.
(compare_tree_sccs_1): Do not compare fields shared in between type
and variant.
(lto_read_decls): Fixup types first before inserting into hash.
Index: lto/lto.c
===
--- lto/lto.c (revision 212159)
+++ lto/lto.c (working copy)
@@ -1050,57 +1050,6 @@ lto_register_function_decl_in_symtab (st
decl, get_resolution (data_in, ix));
}
-/* Copy fields that are not streamed but copied from other nodes. */
-static void
-lto_copy_fields_not_streamed (tree t)
-{
- if (TYPE_P (t) TYPE_MAIN_VARIANT (t) != t)
-{
- tree mv = TYPE_MAIN_VARIANT (t);
-
- if (COMPLETE_TYPE_P (t))
- {
- TYPE_SIZE (t) = TYPE_SIZE (mv);
- TYPE_SIZE_UNIT (t) = TYPE_SIZE_UNIT (mv);
- }
- TYPE_ATTRIBUTES (t) = TYPE_ATTRIBUTES (mv);
-
- if (CODE_CONTAINS_STRUCT (TREE_CODE (t), TS_TYPE_NON_COMMON))
- {
- if (TREE_CODE (t) == ENUMERAL_TYPE COMPLETE_TYPE_P (t))
- TYPE_VALUES (t) = TYPE_VALUES (mv);
- else if (TREE_CODE (t) == ARRAY_TYPE)
- TYPE_DOMAIN (t) = TYPE_DOMAIN (mv);
-
- if (RECORD_OR_UNION_TYPE_P (t) COMPLETE_TYPE_P (t))
- TYPE_VFIELD (t) = TYPE_VFIELD (mv);
- else if ((TREE_CODE (t) == ENUMERAL_TYPE COMPLETE_TYPE_P (t))
- || TREE_CODE (t) == INTEGER_TYPE
- || TREE_CODE (t) == BOOLEAN_TYPE
- || TREE_CODE (t) == REAL_TYPE
- || TREE_CODE (t) == FIXED_POINT_TYPE)
- TYPE_MIN_VALUE (t) = TYPE_MIN_VALUE (mv);
-
- if (TREE_CODE (t) == METHOD_TYPE)
- TYPE_METHOD_BASETYPE (t) = TYPE_METHOD_BASETYPE (mv);
- else if (RECORD_OR_UNION_TYPE_P (t) COMPLETE_TYPE_P (t))
- TYPE_METHODS (t) = TYPE_METHODS (mv);
- else if (TREE_CODE (t) == OFFSET_TYPE)
- TYPE_OFFSET_BASETYPE (t) = TYPE_OFFSET_BASETYPE (mv);
- else if (TREE_CODE (t) == ARRAY_TYPE)
- TYPE_ARRAY_MAX_SIZE (t) = TYPE_ARRAY_MAX_SIZE (mv);
- else if ((TREE_CODE (t) == ENUMERAL_TYPE COMPLETE_TYPE_P (t))
- || TREE_CODE (t) == INTEGER_TYPE
- || TREE_CODE (t) == BOOLEAN_TYPE
- || TREE_CODE (t) == REAL_TYPE
- || TREE_CODE (t) == FIXED_POINT_TYPE)
- TYPE_MAX_VALUE (t) = TYPE_MAX_VALUE (mv);
-
- if (RECORD_OR_UNION_TYPE_P (t) COMPLETE_TYPE_P (t))
- TYPE_BINFO (t) = TYPE_BINFO (mv);
- }
-}
-}
/* For the type T re-materialize it in the type variant list and
the pointer/reference-to chains. */
@@ -1597,28 +1546,15 @@ compare_tree_sccs_1 (tree t1, tree t2, t
if (CODE_CONTAINS_STRUCT (code, TS_TYPE_COMMON))
{
- /* See if type is the main variant. */
- if (TYPE_MAIN_VARIANT (t1) == t1)
- {
- /* Main variant can match only another main variant. */
- if
Re: [PATCH] IPA REF: alias refactoring
gcc/ChangeLog: * cgraph.h (iterate_direct_aliases): New function. (FOR_EACH_ALIAS): New macro iterates all direct aliases for a node. * cgraph.c (cgraph_for_node_thunks_and_aliases): Usage of FOR_EACH_ALIAS added. (cgraph_for_node_and_aliases): Likewise. * cgraphunit.c (assemble_thunks_and_aliases): Likewise. * ipa-inline.c (reset_edge_caches): Likewise. (update_caller_keys): Likewise. * trans-mem.c (ipa_tm_execute): Likewise. *varpool.c (varpool_analyze_node): Likewise. (varpool_for_node_and_aliases): Likewise. * ipa-ref.h (first_alias): New function. (last_alias): Likewise. (has_aliases_p): Likewise. * ipa-ref.c (ipa_ref::remove_reference): Removal function is sensitive to IPA_REF_ALIASes. * symtab.c (symtab_node::add_reference): Node of IPA_REF_ALIAS type are put at the beginning of the list. (symtab_node::iterate_direct_aliases): New function. gcc/lto/ChangeLog: * lto-partition.c (add_symbol_to_partition_1): Usage of FOR_EACH_ALIAS added. OK, thanks! Honza
Re: [GOOGLE] Report the difference between profiled and guessed or annotated branch probabilities.
Fixed. (outputting only the integer percentage)
On Mon, Jun 30, 2014 at 2:20 PM, Yi Yang ahyan...@google.com wrote:
This is intermediate result, which is meant to be consumed by further
post-processing. For this reason I'd prefer to put a number without
that percentage sign.
I'd just output (int)(probability*1+0.5). Does this look good
for you? Or maybe change that to 100 since six digits are more
than enough. I don't see a reason to intentionally drop precision
though.
Note that for the actual probability, the best way to store it is to
store the edge count, since the probability is just
edge_count/bb_count. But this causes disparity in the formats of the
two probabilities.
On Mon, Jun 30, 2014 at 2:12 PM, Dehao Chen de...@google.com wrote:
Let's use %d to replace %f (manual conversion, let's do xx%).
Dehao
On Mon, Jun 30, 2014 at 2:06 PM, Yi Yang ahyan...@google.com wrote:
Fixed.
Also, I spotted some warnings caused by me using %lfs in snprintf().
I changed these to %f and tested.
On Mon, Jun 30, 2014 at 1:49 PM, Dehao Chen de...@google.com wrote:
You don't need extra space to store file name in locus_information_t.
Use pointer instead.
Dehao
On Mon, Jun 30, 2014 at 1:36 PM, Yi Yang ahyan...@google.com wrote:
I refactored the code and added comments. A bug (prematurely breaking
from a loop) was fixed during the refactoring.
(My last mail was wrongly set to HTML instead of plain text. I
apologize for that.)
2014-06-30 Yi Yang ahyan...@google.com
* auto-profile.c (get_locus_information)
(fill_invalid_locus_information, record_branch_prediction_results)
(afdo_calculate_branch_prob, afdo_annotate_cfg): Main comparison and
reporting logic.
* cfg-flags.def (PREDICTED_BY_EXPECT): Add an extra flag representing
an edge's probability is predicted by annotations.
* predict.c (combine_predictions_for_bb): Set up the extra flag on an
edge when appropriate.
* common.opt (fcheck-branch-annotation)
(fcheck-branch-annotation-threshold=): Add an extra GCC option to turn
on report
On Fri, Jun 27, 2014 at 3:20 PM, Xinliang David Li davi...@google.com
wrote:
Hi Yi,
1) please add comments before new functions as documentation -- follow
the coding style guideline
2) missing documenation on the new flags (pointed out by Gerald)
3) Please refactor the check code in afdo_calculate_branch_prob into a
helper function
4) the change log is not needed for google branches, but if provided,
the format should follow the style guide (e.g, function name in () ).
David
On Fri, Jun 27, 2014 at 11:07 AM, Yi Yang ahyan...@google.com wrote:
Hi,
This patch adds an option. When the option is enabled, GCC will add a
record about it in an elf section called
.gnu.switches.text.branch.annotation for every branch.
gcc/
2014-06-27 Yi Yang ahyan...@google.com
* auto-profile.c: Main comparison and reporting logic.
* cfg-flags.def: Add an extra flag representing an edge's
probability is predicted by annotations.
* predict.c: Set up the extra flag on an edge when appropriate.
* common.opt: Add an extra GCC option to turn on this report
mechanism
diff --git gcc/auto-profile.c gcc/auto-profile.c
index 74d3d1d..d8901b9 100644
--- gcc/auto-profile.c
+++ gcc/auto-profile.c
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3. If not see
#include opts.h/* for in_fnames. */
#include tree-pass.h /* for ipa pass. */
#include cfgloop.h /* for loop_optimizer_init. */
+#include md5.h /* for hashing function names. */
#include gimple.h
#include cgraph.h
#include tree-flow.h
@@ -1367,6 +1368,148 @@ afdo_propagate (void)
}
}
+/* All information parsed from a location_t that will be stored into the ELF
+ section. */
+
+struct locus_information_t {
+ /* File name of the source file containing the branch. */
+ const char *filename;
+ /* Line number of the branch location. */
+ unsigned lineno;
+ /* Line number of the first line of function definition. */
+ unsigned function_lineno;
+ /* Hash value calculated from function name and length, used to uniquely
+ identify a function across different source versions. */
+ char function_hash[33];
+};
+
+/* Return true iff file and lineno are available for the provided locus.
+ Fill all fields of li with information about locus. */
+
+static bool
+get_locus_information (location_t locus, locus_information_t* li) {
+ if (locus == UNKNOWN_LOCATION || !LOCATION_FILE (locus))
+return false;
+ li-filename = LOCATION_FILE (locus);
+ li-lineno = LOCATION_LINE (locus);
+
+ tree block = LOCATION_BLOCK (locus);
+ tree function_decl = current_function_decl;
+
+ if (block TREE_CODE (block) == BLOCK)
+{
+ for (block = BLOCK_SUPERCONTEXT (block);
+ block (TREE_CODE (block) == BLOCK);
+ block = BLOCK_SUPERCONTEXT
[C++] make TYPE_DECLs public for builtin type and templates
Jason,
I made a verifier that types not considered anonymous are not built
from types that are anonymous and that public types have TREE_PUBLIC
flag on their TYPE_DECLS (as that seem to be what tree.h says).
This catches two cases in C++ FE and several other cases elsewhere.
Does something like this make sense? I think bulitin_types are
generally public, but I am not quite sure about templates.
Honza
* decl.c (record_builtin_type): Make builting types public.
* pt.c (reduce_template_parm_level): Make TYPE_DECL public.
Index: decl.c
===
--- decl.c (revision 212098)
+++ decl.c (working copy)
@@ -3591,6 +3591,7 @@ record_builtin_type (enum rid rid_index,
tdecl = build_decl (BUILTINS_LOCATION, TYPE_DECL, tname, type);
DECL_ARTIFICIAL (tdecl) = 1;
SET_IDENTIFIER_GLOBAL_VALUE (tname, tdecl);
+ TREE_PUBLIC (tdecl) = true;
}
if (rname)
{
@@ -3598,6 +3599,7 @@ record_builtin_type (enum rid rid_index,
{
tdecl = build_decl (BUILTINS_LOCATION, TYPE_DECL, rname, type);
DECL_ARTIFICIAL (tdecl) = 1;
+ TREE_PUBLIC (tdecl) = true;
}
SET_IDENTIFIER_GLOBAL_VALUE (rname, tdecl);
}
Index: pt.c
===
--- pt.c(revision 212098)
+++ pt.c(working copy)
@@ -3623,6 +3623,8 @@ reduce_template_parm_level (tree index,
TREE_CODE (orig_decl), DECL_NAME (orig_decl), type);
TREE_CONSTANT (decl) = TREE_CONSTANT (orig_decl);
TREE_READONLY (decl) = TREE_READONLY (orig_decl);
+ if (TREE_CODE (decl) == TYPE_DECL)
+TREE_PUBLIC (decl) = 1;
DECL_ARTIFICIAL (decl) = 1;
SET_DECL_TEMPLATE_PARM_P (decl);
Make TYPE_DECLs public in common code
Jason,
this is non-C++ specific part where I needed updating. The i386.c change will
need to be propagated into other backends. I also wonder if I should copy
PUBLIC flag from component type in complex numbers - can we produce complex
number from anonymous type?
Finally type_in_anonymous_namespace_p needs to be extended into two cases where
the type is considered nonanonymous by lack of DECL_NAME.
Bootstrapped/regtested x86_64-linux, seems sane?
Honza
* config/i386/i386.c (ix86_build_builtin_va_list_abi): Make va_list_tag
type name public
* stor-layout.c (finish_builtin_struct): Make builtin types public.
* tree.c (build_complex_type): Complex types are public.
(type_in_anonymous_namespace_p): Handle methods and arrays.
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 212098)
+++ config/i386/i386.c (working copy)
@@ -8120,6 +8120,7 @@ ix86_build_builtin_va_list_abi (enum cal
record = lang_hooks.types.make_type (RECORD_TYPE);
type_decl = build_decl (BUILTINS_LOCATION,
TYPE_DECL, get_identifier (__va_list_tag), record);
+ TREE_PUBLIC (type_decl) = 1;
f_gpr = build_decl (BUILTINS_LOCATION,
FIELD_DECL, get_identifier (gp_offset),
Index: stor-layout.c
===
--- stor-layout.c (revision 212106)
+++ stor-layout.c (working copy)
@@ -2092,6 +2092,7 @@ finish_builtin_struct (tree type, const
TYPE_NAME (type) = build_decl (BUILTINS_LOCATION,
TYPE_DECL, get_identifier (name), type);
#endif
+ TREE_PUBLIC (TYPE_NAME (type)) = true;
TYPE_STUB_DECL (type) = TYPE_NAME (type);
layout_decl (TYPE_NAME (type), 0);
}
Index: tree.c
===
--- tree.c (revision 212098)
+++ tree.c (working copy)
@@ -8360,8 +8394,11 @@ build_complex_type (tree component_type)
name = 0;
if (name != 0)
- TYPE_NAME (t) = build_decl (UNKNOWN_LOCATION, TYPE_DECL,
- get_identifier (name), t);
+ {
+ TYPE_NAME (t) = build_decl (UNKNOWN_LOCATION, TYPE_DECL,
+ get_identifier (name), t);
+ TREE_PUBLIC (TYPE_NAME (t)) = true;
+ }
}
return build_qualified_type (t, TYPE_QUALS (component_type));
@@ -12009,8 +11901,16 @@ obj_type_ref_class (tree ref)
/* Return true if T is in anonymous namespace. */
bool
-type_in_anonymous_namespace_p (tree t)
+type_in_anonymous_namespace_p (const_tree t)
{
+ /* Methods do not have their STUB_DECLs, but they are anonymous
+ if type they belong to is. */
+ if (TREE_CODE (t) == METHOD_TYPE)
+t = TYPE_METHOD_BASETYPE (t);
+ /* Arrays do not need to have STUB_DECLs, so look for the base
+ type. */
+ while (!TYPE_STUB_DECL (t) TREE_CODE (t) == ARRAY_TYPE)
+t = TREE_TYPE (t);
return (TYPE_STUB_DECL (t) !TREE_PUBLIC (TYPE_STUB_DECL (t)));
}
Re: [PATCH] Don't ICE with huge alignment (PR middle-end/60226)
On Jun 30, 2014, at 12:50 PM, Jeff Law l...@redhat.com wrote:
On 03/04/14 09:40, Marek Polacek wrote:
This should fix ICE on insane alignment. Normally, check_user_alignment
detects e.g. alignment 1 32, but not 1 28. However, record_align
is in bits, so it's actually 8 * (1 28) and that's greater than
INT_MAX. This patch rejects such code.
In the middle hunk, we should give up when an error occurs, we don't
want to call finalize_type_size in that case -- we'd ICE in there.
Regtested/bootstrapped on x86_64-linux, ok for trunk?
2014-03-04 Marek Polacek pola...@redhat.com
PR middle-end/60226
* stor-layout.c (layout_type): Return if alignment of array elements
is greater than element size. Error out if requested alignment is too
large.
cp/
* class.c (layout_class_type): Error out if requested alignment is too
large.
testsuite/
* c-c++-common/pr60226.c: New test.
Is this still applicable after the wide-int changes? I haven't looked
closely.
I glanced at it:
(gdb) p/x TYPE_ALIGN (type)
$1 = 2147483648
(gdb) p/x TYPE_ALIGN (type)
$2 = 0x8000
The callee is int, the caller uses unsigned int. The assert I see is because
the routines are not type correct:
=TYPE_SIZE (type) = round_up (TYPE_SIZE (type), TYPE_ALIGN (type));
(gdb) ptype TYPE_ALIGN (type)
type = unsigned int
tree
round_up_loc (location_t loc, tree value, int divisor)
{
tree div = NULL_TREE;
=gcc_assert (divisor 0);
Would be nice if the routine was type correct (wrt unsigned).
Re: [PATCH] Don't ICE with huge alignment (PR middle-end/60226)
On Jun 30, 2014, at 3:40 PM, Mike Stump mikest...@comcast.net wrote: Is this still applicable after the wide-int changes? I haven't looked closely. Oops, forgot to state what I wanted to state… Yes, it still aborts post wide-int…
Re: [DOC Patch] Explicit Register Variables
On 6/30/2014 2:01 PM, Jeff Law wrote: On 06/30/14 02:18, David Wohlferd wrote: I don't have permissions to commit this patch, but I do have a release on file with the FSF. Problem description: The text for using Explicit Register Variables is confusing, redundant, and fails to make certain essential information clear. Some specific problems: - There is no reason to call this topic Explicit Reg Vars instead of Explicit Register Variables. Agreed. - Putting text on the Explicit Register Variables menu page (instead of the Global or Local subpages) is redundant, since any text that describes the usage of Global or Local register variables will have to be repeated on the individual subpages. OK. - Vague promises of features that may some day be available are not useful in documentation (Eventually there may be a way of asking the compiler to). Can't disagree here. I believe this lame comment has been around for 20 years or longer at this point. - Vague descriptions of things that are reported to work on certain platforms are not useful (On the SPARC, there are reports that). I'd disagree. But what's more important here is the registers that are available are a function of the ABI and for someone to attempt to use this feature, they're going to have to be intimately aware of the ABI of their target. If we could say On the SPARC, these registers can be used for I'd be tempted to leave it as an example. But saying Well, someone once said they thought it might work this way on this one specific platform is not helpful for either SPARC or non-SPARC users. - Describing Local Register Variables as sometimes convenient for use with the extended asm feature misses the point that this is in fact the ONLY supported use for Local Register Variables. What makes you believe that this feature is only useful for extended asms? My recollection is that for the last 20+ years this feature has served effectively as a pre-allocation of certain function scoped objects into predetermined registers. That's great feedback. The existing docs don't even mention that use. If you have a link (or if you can put together a few sentences) describing how you could use it for this, I'll add it and re-work the text. And if you can think of any other common uses, I'd be glad to add them too. - Unambiguous statements such as The following uses are explicitly /not/ supported when describing things such as calling Basic asm discourage people from attempting to misuse the feature. Not sure I really agree with this either. There's nothing inherently wrong with someone trying to use a local register variable to try and optimize code for example. Umm. Well. Hmm. You haven't said this was a good idea or even that it would work, so I guess the statement that there's nothing inherently wrong with it is true. However, by explicitly stating it is not supported, we would be making it clear that when the next release uses registers in a slightly different way that wipes out all their improvements, we can say we told you that wasn't a supported use of this feature. However, I'm prepared to remove or rework this statement if you feel strongly. ChangeLog: 2014-06-30 David Wohlferd d...@limegreensocks.com * doc/extend.texi (Explicit Reg Vars): Rewrite and clarify. * doc/implement-c.texi (Hints implementation): Use new name. So in summary, I agree with most of what you've done. The question is how to proceed at this point. We could extract the non-controversial stuff and install it now and iterate on the stuff still in the air until it's done. Or we could iterate on the stuff still in the air and install the final update as an single commit. I don't care much either way, do you have a preference? My preference would be to only do one checkin when we've got it right. I *almost* sent this as an RFD to the gcc list. But it being such an obscure area, I wasn't really expecting much response (see https://gcc.gnu.org/ml/gcc-help/2014-04/msg00124.html for example). The items listed in my problem description were only highlights. As you have no doubt noticed, nearly all the text on these 2 pages got modified. If you have feedback on any of the rest of the text, I'd love to hear it. dw
Re: [GOOGLE] Report the difference between profiled and guessed or annotated branch probabilities.
Instead of storing percentages of the branch probabilities, store them
times REG_BR_PROB_BASE.
On Mon, Jun 30, 2014 at 3:26 PM, Yi Yang ahyan...@google.com wrote:
Fixed. (outputting only the integer percentage)
On Mon, Jun 30, 2014 at 2:20 PM, Yi Yang ahyan...@google.com wrote:
This is intermediate result, which is meant to be consumed by further
post-processing. For this reason I'd prefer to put a number without
that percentage sign.
I'd just output (int)(probability*1+0.5). Does this look good
for you? Or maybe change that to 100 since six digits are more
than enough. I don't see a reason to intentionally drop precision
though.
Note that for the actual probability, the best way to store it is to
store the edge count, since the probability is just
edge_count/bb_count. But this causes disparity in the formats of the
two probabilities.
On Mon, Jun 30, 2014 at 2:12 PM, Dehao Chen de...@google.com wrote:
Let's use %d to replace %f (manual conversion, let's do xx%).
Dehao
On Mon, Jun 30, 2014 at 2:06 PM, Yi Yang ahyan...@google.com wrote:
Fixed.
Also, I spotted some warnings caused by me using %lfs in snprintf().
I changed these to %f and tested.
On Mon, Jun 30, 2014 at 1:49 PM, Dehao Chen de...@google.com wrote:
You don't need extra space to store file name in locus_information_t.
Use pointer instead.
Dehao
On Mon, Jun 30, 2014 at 1:36 PM, Yi Yang ahyan...@google.com wrote:
I refactored the code and added comments. A bug (prematurely breaking
from a loop) was fixed during the refactoring.
(My last mail was wrongly set to HTML instead of plain text. I
apologize for that.)
2014-06-30 Yi Yang ahyan...@google.com
* auto-profile.c (get_locus_information)
(fill_invalid_locus_information, record_branch_prediction_results)
(afdo_calculate_branch_prob, afdo_annotate_cfg): Main comparison and
reporting logic.
* cfg-flags.def (PREDICTED_BY_EXPECT): Add an extra flag representing
an edge's probability is predicted by annotations.
* predict.c (combine_predictions_for_bb): Set up the extra flag on an
edge when appropriate.
* common.opt (fcheck-branch-annotation)
(fcheck-branch-annotation-threshold=): Add an extra GCC option to
turn
on report
On Fri, Jun 27, 2014 at 3:20 PM, Xinliang David Li davi...@google.com
wrote:
Hi Yi,
1) please add comments before new functions as documentation -- follow
the coding style guideline
2) missing documenation on the new flags (pointed out by Gerald)
3) Please refactor the check code in afdo_calculate_branch_prob into a
helper function
4) the change log is not needed for google branches, but if provided,
the format should follow the style guide (e.g, function name in () ).
David
On Fri, Jun 27, 2014 at 11:07 AM, Yi Yang ahyan...@google.com wrote:
Hi,
This patch adds an option. When the option is enabled, GCC will add a
record about it in an elf section called
.gnu.switches.text.branch.annotation for every branch.
gcc/
2014-06-27 Yi Yang ahyan...@google.com
* auto-profile.c: Main comparison and reporting logic.
* cfg-flags.def: Add an extra flag representing an edge's
probability is predicted by annotations.
* predict.c: Set up the extra flag on an edge when
appropriate.
* common.opt: Add an extra GCC option to turn on this report
mechanism
diff --git gcc/auto-profile.c gcc/auto-profile.c
index 74d3d1d..dc159e7 100644
--- gcc/auto-profile.c
+++ gcc/auto-profile.c
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3. If not see
#include opts.h/* for in_fnames. */
#include tree-pass.h /* for ipa pass. */
#include cfgloop.h /* for loop_optimizer_init. */
+#include md5.h /* for hashing function names. */
#include gimple.h
#include cgraph.h
#include tree-flow.h
@@ -1367,6 +1368,147 @@ afdo_propagate (void)
}
}
+/* All information parsed from a location_t that will be stored into the ELF
+ section. */
+
+struct locus_information_t {
+ /* File name of the source file containing the branch. */
+ const char *filename;
+ /* Line number of the branch location. */
+ unsigned lineno;
+ /* Line number of the first line of function definition. */
+ unsigned function_lineno;
+ /* Hash value calculated from function name and length, used to uniquely
+ identify a function across different source versions. */
+ char function_hash[33];
+};
+
+/* Return true iff file and lineno are available for the provided locus.
+ Fill all fields of li with information about locus. */
+
+static bool
+get_locus_information (location_t locus, locus_information_t* li) {
+ if (locus == UNKNOWN_LOCATION || !LOCATION_FILE (locus))
+return false;
+ li-filename = LOCATION_FILE (locus);
+ li-lineno = LOCATION_LINE (locus);
+
+ tree block = LOCATION_BLOCK (locus);
+ tree function_decl = current_function_decl;
+
+ if (block
Re: [GOOGLE] Report the difference between profiled and guessed or annotated branch probabilities.
There is no need for fill_invalid_locus_information, just initialize every field to 0, and if it's unknown location, no need to output this line. Dehao On Mon, Jun 30, 2014 at 4:26 PM, Yi Yang ahyan...@google.com wrote: Instead of storing percentages of the branch probabilities, store them times REG_BR_PROB_BASE. On Mon, Jun 30, 2014 at 3:26 PM, Yi Yang ahyan...@google.com wrote: Fixed. (outputting only the integer percentage) On Mon, Jun 30, 2014 at 2:20 PM, Yi Yang ahyan...@google.com wrote: This is intermediate result, which is meant to be consumed by further post-processing. For this reason I'd prefer to put a number without that percentage sign. I'd just output (int)(probability*1+0.5). Does this look good for you? Or maybe change that to 100 since six digits are more than enough. I don't see a reason to intentionally drop precision though. Note that for the actual probability, the best way to store it is to store the edge count, since the probability is just edge_count/bb_count. But this causes disparity in the formats of the two probabilities. On Mon, Jun 30, 2014 at 2:12 PM, Dehao Chen de...@google.com wrote: Let's use %d to replace %f (manual conversion, let's do xx%). Dehao On Mon, Jun 30, 2014 at 2:06 PM, Yi Yang ahyan...@google.com wrote: Fixed. Also, I spotted some warnings caused by me using %lfs in snprintf(). I changed these to %f and tested. On Mon, Jun 30, 2014 at 1:49 PM, Dehao Chen de...@google.com wrote: You don't need extra space to store file name in locus_information_t. Use pointer instead. Dehao On Mon, Jun 30, 2014 at 1:36 PM, Yi Yang ahyan...@google.com wrote: I refactored the code and added comments. A bug (prematurely breaking from a loop) was fixed during the refactoring. (My last mail was wrongly set to HTML instead of plain text. I apologize for that.) 2014-06-30 Yi Yang ahyan...@google.com * auto-profile.c (get_locus_information) (fill_invalid_locus_information, record_branch_prediction_results) (afdo_calculate_branch_prob, afdo_annotate_cfg): Main comparison and reporting logic. * cfg-flags.def (PREDICTED_BY_EXPECT): Add an extra flag representing an edge's probability is predicted by annotations. * predict.c (combine_predictions_for_bb): Set up the extra flag on an edge when appropriate. * common.opt (fcheck-branch-annotation) (fcheck-branch-annotation-threshold=): Add an extra GCC option to turn on report On Fri, Jun 27, 2014 at 3:20 PM, Xinliang David Li davi...@google.com wrote: Hi Yi, 1) please add comments before new functions as documentation -- follow the coding style guideline 2) missing documenation on the new flags (pointed out by Gerald) 3) Please refactor the check code in afdo_calculate_branch_prob into a helper function 4) the change log is not needed for google branches, but if provided, the format should follow the style guide (e.g, function name in () ). David On Fri, Jun 27, 2014 at 11:07 AM, Yi Yang ahyan...@google.com wrote: Hi, This patch adds an option. When the option is enabled, GCC will add a record about it in an elf section called .gnu.switches.text.branch.annotation for every branch. gcc/ 2014-06-27 Yi Yang ahyan...@google.com * auto-profile.c: Main comparison and reporting logic. * cfg-flags.def: Add an extra flag representing an edge's probability is predicted by annotations. * predict.c: Set up the extra flag on an edge when appropriate. * common.opt: Add an extra GCC option to turn on this report mechanism
Re: [GOOGLE] Report the difference between profiled and guessed or annotated branch probabilities.
Removed fill_invalid_locus_information. Change the function call to a
return statement.
On Mon, Jun 30, 2014 at 4:42 PM, Dehao Chen de...@google.com wrote:
There is no need for fill_invalid_locus_information, just initialize
every field to 0, and if it's unknown location, no need to output this
line.
Dehao
On Mon, Jun 30, 2014 at 4:26 PM, Yi Yang ahyan...@google.com wrote:
Instead of storing percentages of the branch probabilities, store them
times REG_BR_PROB_BASE.
On Mon, Jun 30, 2014 at 3:26 PM, Yi Yang ahyan...@google.com wrote:
Fixed. (outputting only the integer percentage)
On Mon, Jun 30, 2014 at 2:20 PM, Yi Yang ahyan...@google.com wrote:
This is intermediate result, which is meant to be consumed by further
post-processing. For this reason I'd prefer to put a number without
that percentage sign.
I'd just output (int)(probability*1+0.5). Does this look good
for you? Or maybe change that to 100 since six digits are more
than enough. I don't see a reason to intentionally drop precision
though.
Note that for the actual probability, the best way to store it is to
store the edge count, since the probability is just
edge_count/bb_count. But this causes disparity in the formats of the
two probabilities.
On Mon, Jun 30, 2014 at 2:12 PM, Dehao Chen de...@google.com wrote:
Let's use %d to replace %f (manual conversion, let's do xx%).
Dehao
On Mon, Jun 30, 2014 at 2:06 PM, Yi Yang ahyan...@google.com wrote:
Fixed.
Also, I spotted some warnings caused by me using %lfs in snprintf().
I changed these to %f and tested.
On Mon, Jun 30, 2014 at 1:49 PM, Dehao Chen de...@google.com wrote:
You don't need extra space to store file name in locus_information_t.
Use pointer instead.
Dehao
On Mon, Jun 30, 2014 at 1:36 PM, Yi Yang ahyan...@google.com wrote:
I refactored the code and added comments. A bug (prematurely breaking
from a loop) was fixed during the refactoring.
(My last mail was wrongly set to HTML instead of plain text. I
apologize for that.)
2014-06-30 Yi Yang ahyan...@google.com
* auto-profile.c (get_locus_information)
(fill_invalid_locus_information, record_branch_prediction_results)
(afdo_calculate_branch_prob, afdo_annotate_cfg): Main comparison
and
reporting logic.
* cfg-flags.def (PREDICTED_BY_EXPECT): Add an extra flag
representing
an edge's probability is predicted by annotations.
* predict.c (combine_predictions_for_bb): Set up the extra flag on
an
edge when appropriate.
* common.opt (fcheck-branch-annotation)
(fcheck-branch-annotation-threshold=): Add an extra GCC option to
turn
on report
On Fri, Jun 27, 2014 at 3:20 PM, Xinliang David Li
davi...@google.com wrote:
Hi Yi,
1) please add comments before new functions as documentation --
follow
the coding style guideline
2) missing documenation on the new flags (pointed out by Gerald)
3) Please refactor the check code in afdo_calculate_branch_prob into
a
helper function
4) the change log is not needed for google branches, but if provided,
the format should follow the style guide (e.g, function name in () ).
David
On Fri, Jun 27, 2014 at 11:07 AM, Yi Yang ahyan...@google.com
wrote:
Hi,
This patch adds an option. When the option is enabled, GCC will add
a
record about it in an elf section called
.gnu.switches.text.branch.annotation for every branch.
gcc/
2014-06-27 Yi Yang ahyan...@google.com
* auto-profile.c: Main comparison and reporting logic.
* cfg-flags.def: Add an extra flag representing an edge's
probability is predicted by annotations.
* predict.c: Set up the extra flag on an edge when
appropriate.
* common.opt: Add an extra GCC option to turn on this
report mechanism
diff --git gcc/auto-profile.c gcc/auto-profile.c
index 74d3d1d..3cf5a74 100644
--- gcc/auto-profile.c
+++ gcc/auto-profile.c
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3. If not see
#include opts.h/* for in_fnames. */
#include tree-pass.h /* for ipa pass. */
#include cfgloop.h /* for loop_optimizer_init. */
+#include md5.h /* for hashing function names. */
#include gimple.h
#include cgraph.h
#include tree-flow.h
@@ -1367,6 +1368,135 @@ afdo_propagate (void)
}
}
+/* All information parsed from a location_t that will be stored into the ELF
+ section. */
+
+struct locus_information_t {
+ /* File name of the source file containing the branch. */
+ const char *filename;
+ /* Line number of the branch location. */
+ unsigned lineno;
+ /* Line number of the first line of function definition. */
+ unsigned function_lineno;
+ /* Hash value calculated from function name and length, used to uniquely
+ identify a function across different source versions. */
+ char function_hash[33];
+};
+
+/* Return true iff file and lineno are available for the provided locus.
+
Re: [GOOGLE] Report the difference between profiled and guessed or annotated branch probabilities.
Done.
On Mon, Jun 30, 2014 at 5:20 PM, Dehao Chen de...@google.com wrote:
For get_locus_information, can you cal get_inline_stack and directly use its
output to get the function name instead?
Dehao
On Mon, Jun 30, 2014 at 4:47 PM, Yi Yang ahyan...@google.com wrote:
Removed fill_invalid_locus_information. Change the function call to a
return statement.
On Mon, Jun 30, 2014 at 4:42 PM, Dehao Chen de...@google.com wrote:
There is no need for fill_invalid_locus_information, just initialize
every field to 0, and if it's unknown location, no need to output this
line.
Dehao
On Mon, Jun 30, 2014 at 4:26 PM, Yi Yang ahyan...@google.com wrote:
Instead of storing percentages of the branch probabilities, store them
times REG_BR_PROB_BASE.
On Mon, Jun 30, 2014 at 3:26 PM, Yi Yang ahyan...@google.com wrote:
Fixed. (outputting only the integer percentage)
On Mon, Jun 30, 2014 at 2:20 PM, Yi Yang ahyan...@google.com wrote:
This is intermediate result, which is meant to be consumed by further
post-processing. For this reason I'd prefer to put a number without
that percentage sign.
I'd just output (int)(probability*1+0.5). Does this look good
for you? Or maybe change that to 100 since six digits are more
than enough. I don't see a reason to intentionally drop precision
though.
Note that for the actual probability, the best way to store it is to
store the edge count, since the probability is just
edge_count/bb_count. But this causes disparity in the formats of the
two probabilities.
On Mon, Jun 30, 2014 at 2:12 PM, Dehao Chen de...@google.com wrote:
Let's use %d to replace %f (manual conversion, let's do xx%).
Dehao
On Mon, Jun 30, 2014 at 2:06 PM, Yi Yang ahyan...@google.com
wrote:
Fixed.
Also, I spotted some warnings caused by me using %lfs in
snprintf().
I changed these to %f and tested.
On Mon, Jun 30, 2014 at 1:49 PM, Dehao Chen de...@google.com
wrote:
You don't need extra space to store file name in
locus_information_t.
Use pointer instead.
Dehao
On Mon, Jun 30, 2014 at 1:36 PM, Yi Yang ahyan...@google.com
wrote:
I refactored the code and added comments. A bug (prematurely
breaking
from a loop) was fixed during the refactoring.
(My last mail was wrongly set to HTML instead of plain text. I
apologize for that.)
2014-06-30 Yi Yang ahyan...@google.com
* auto-profile.c (get_locus_information)
(fill_invalid_locus_information,
record_branch_prediction_results)
(afdo_calculate_branch_prob, afdo_annotate_cfg): Main
comparison and
reporting logic.
* cfg-flags.def (PREDICTED_BY_EXPECT): Add an extra flag
representing
an edge's probability is predicted by annotations.
* predict.c (combine_predictions_for_bb): Set up the extra
flag on an
edge when appropriate.
* common.opt (fcheck-branch-annotation)
(fcheck-branch-annotation-threshold=): Add an extra GCC
option to turn
on report
On Fri, Jun 27, 2014 at 3:20 PM, Xinliang David Li
davi...@google.com wrote:
Hi Yi,
1) please add comments before new functions as documentation --
follow
the coding style guideline
2) missing documenation on the new flags (pointed out by
Gerald)
3) Please refactor the check code in afdo_calculate_branch_prob
into a
helper function
4) the change log is not needed for google branches, but if
provided,
the format should follow the style guide (e.g, function name in
() ).
David
On Fri, Jun 27, 2014 at 11:07 AM, Yi Yang ahyan...@google.com
wrote:
Hi,
This patch adds an option. When the option is enabled, GCC
will add a
record about it in an elf section called
.gnu.switches.text.branch.annotation for every branch.
gcc/
2014-06-27 Yi Yang ahyan...@google.com
* auto-profile.c: Main comparison and reporting logic.
* cfg-flags.def: Add an extra flag representing an
edge's
probability is predicted by annotations.
* predict.c: Set up the extra flag on an edge when
appropriate.
* common.opt: Add an extra GCC option to turn on this
report mechanism
diff --git gcc/auto-profile.c gcc/auto-profile.c
index 74d3d1d..5b71fb0 100644
--- gcc/auto-profile.c
+++ gcc/auto-profile.c
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3. If not see
#include opts.h/* for in_fnames. */
#include tree-pass.h /* for ipa pass. */
#include cfgloop.h /* for loop_optimizer_init. */
+#include md5.h /* for hashing function names. */
#include gimple.h
#include cgraph.h
#include tree-flow.h
@@ -1367,6 +1368,125 @@ afdo_propagate (void)
}
}
+/* All information parsed from a location_t that will be stored into the ELF
+ section. */
+
+struct locus_information_t {
+ /* File name of the source file containing the branch. */
+ const char *filename;
+
Re: [GOOGLE] Report the difference between profiled and guessed or annotated branch probabilities.
Fixed a style error (missing a space before a left parenthesis) On Mon, Jun 30, 2014 at 5:41 PM, Yi Yang ahyan...@google.com wrote: Done. On Mon, Jun 30, 2014 at 5:20 PM, Dehao Chen de...@google.com wrote: For get_locus_information, can you cal get_inline_stack and directly use its output to get the function name instead? Dehao On Mon, Jun 30, 2014 at 4:47 PM, Yi Yang ahyan...@google.com wrote: Removed fill_invalid_locus_information. Change the function call to a return statement. On Mon, Jun 30, 2014 at 4:42 PM, Dehao Chen de...@google.com wrote: There is no need for fill_invalid_locus_information, just initialize every field to 0, and if it's unknown location, no need to output this line. Dehao On Mon, Jun 30, 2014 at 4:26 PM, Yi Yang ahyan...@google.com wrote: Instead of storing percentages of the branch probabilities, store them times REG_BR_PROB_BASE. On Mon, Jun 30, 2014 at 3:26 PM, Yi Yang ahyan...@google.com wrote: Fixed. (outputting only the integer percentage) On Mon, Jun 30, 2014 at 2:20 PM, Yi Yang ahyan...@google.com wrote: This is intermediate result, which is meant to be consumed by further post-processing. For this reason I'd prefer to put a number without that percentage sign. I'd just output (int)(probability*1+0.5). Does this look good for you? Or maybe change that to 100 since six digits are more than enough. I don't see a reason to intentionally drop precision though. Note that for the actual probability, the best way to store it is to store the edge count, since the probability is just edge_count/bb_count. But this causes disparity in the formats of the two probabilities. On Mon, Jun 30, 2014 at 2:12 PM, Dehao Chen de...@google.com wrote: Let's use %d to replace %f (manual conversion, let's do xx%). Dehao On Mon, Jun 30, 2014 at 2:06 PM, Yi Yang ahyan...@google.com wrote: Fixed. Also, I spotted some warnings caused by me using %lfs in snprintf(). I changed these to %f and tested. On Mon, Jun 30, 2014 at 1:49 PM, Dehao Chen de...@google.com wrote: You don't need extra space to store file name in locus_information_t. Use pointer instead. Dehao On Mon, Jun 30, 2014 at 1:36 PM, Yi Yang ahyan...@google.com wrote: I refactored the code and added comments. A bug (prematurely breaking from a loop) was fixed during the refactoring. (My last mail was wrongly set to HTML instead of plain text. I apologize for that.) 2014-06-30 Yi Yang ahyan...@google.com * auto-profile.c (get_locus_information) (fill_invalid_locus_information, record_branch_prediction_results) (afdo_calculate_branch_prob, afdo_annotate_cfg): Main comparison and reporting logic. * cfg-flags.def (PREDICTED_BY_EXPECT): Add an extra flag representing an edge's probability is predicted by annotations. * predict.c (combine_predictions_for_bb): Set up the extra flag on an edge when appropriate. * common.opt (fcheck-branch-annotation) (fcheck-branch-annotation-threshold=): Add an extra GCC option to turn on report On Fri, Jun 27, 2014 at 3:20 PM, Xinliang David Li davi...@google.com wrote: Hi Yi, 1) please add comments before new functions as documentation -- follow the coding style guideline 2) missing documenation on the new flags (pointed out by Gerald) 3) Please refactor the check code in afdo_calculate_branch_prob into a helper function 4) the change log is not needed for google branches, but if provided, the format should follow the style guide (e.g, function name in () ). David On Fri, Jun 27, 2014 at 11:07 AM, Yi Yang ahyan...@google.com wrote: Hi, This patch adds an option. When the option is enabled, GCC will add a record about it in an elf section called .gnu.switches.text.branch.annotation for every branch. gcc/ 2014-06-27 Yi Yang ahyan...@google.com * auto-profile.c: Main comparison and reporting logic. * cfg-flags.def: Add an extra flag representing an edge's probability is predicted by annotations. * predict.c: Set up the extra flag on an edge when appropriate. * common.opt: Add an extra GCC option to turn on this report mechanism diff --git gcc/auto-profile.c gcc/auto-profile.c index 74d3d1d..3122835 100644 --- gcc/auto-profile.c +++ gcc/auto-profile.c @@ -40,6 +40,7 @@ along with GCC; see the file COPYING3. If not see #include opts.h/* for in_fnames. */ #include tree-pass.h /* for ipa pass. */ #include cfgloop.h /* for loop_optimizer_init. */ +#include md5.h /* for hashing function names. */ #include gimple.h #include cgraph.h #include tree-flow.h @@ -1367,6 +1368,125 @@ afdo_propagate (void) } } +/* All information parsed from a location_t that will be stored into the ELF +
Re: [patch libstdc++] Add xmethods for std::vector and std::unique_ptr
On Mon, Jun 30, 2014 at 8:00 AM, Tom Tromey tro...@redhat.com wrote:
Siva == Siva Chandra sivachan...@google.com writes:
Siva +# Load the xmethods.
Siva +from libstdcxx.v6.xmethods import register_libstdcxx_xmethods
Siva +register_libstdcxx_xmethods (gdb.current_objfile ())
I don't think any addition to the hook file should be needed.
I removed it in the attached patch.
Siva +# The object to be returned is the 0-th element in the tuple
_m_t.
Siva +# The following retrieves the 0-th element of this tuple.
Siva +_m_t_base = _m_t[_m_t.type.fields()[0]] # std::tuple has a
single base
Siva +# class and no data
members.
Siva +for f in _m_t_base.type.fields():
Siva +# The object is embedded in the _Head_base base class of
Siva +# _m_t_base.
Siva +if f.is_base_class and f.name.find('std::_Head_base') ==
0:
Siva +_head_base = _m_t_base[f]
Did you investigate sharing any code with the printers?
If so, why did you choose not to?
If not, could you please look into that?
I have considered this. I tried to be too smart in the above snippet.
But, look at the attached patch; All that can be replaced by a simple
one-liner. In which case, would trying to make it common for
prettyprinters and xmethods become an overkill? For std::vector, I do
not think there is an overlap between xmethods and prettyprinters. I
will keep this in mind for future xmethods though.
ChangeLog
libstdc++-v3/
2014-06-30 Siva Chandra Reddy sivachan...@google.com
* python/libstdcxx/v6/xmethods.py: New file.
* testsuite/lib/gdb-test.exp (gdb_version_check_xmethods): New
function.
(gdb-test): New optional argument LOAD_XMETHODS. Load xmethods
python script if LOAD_XMETHODS is true.
* testsuite/libstdc++-xmethods/unique_ptr.cc: New file.
* testsuite/libstdc++-xmethods/vector.cc: New file.
* testsuite/libstdc++-xmethods/xmethods.exp: New file.
diff --git a/libstdc++-v3/python/libstdcxx/v6/xmethods.py
b/libstdc++-v3/python/libstdcxx/v6/xmethods.py
new file mode 100644
index 000..f20f411
--- /dev/null
+++ b/libstdc++-v3/python/libstdcxx/v6/xmethods.py
@@ -0,0 +1,103 @@
+# Xmethods for libstc++.
+
+# Copyright (C) 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see http://www.gnu.org/licenses/.
+
+import gdb
+import gdb.xmethod
+import re
+
+matcher_name_prefix = 'libstdc++::'
+
+# Xmethods for std::vector
+
+class VectorSizeWorker(gdb.xmethod.XMethodWorker):
+def __init__(self):
+self.name = 'size'
+self.enabled = True
+
+def get_arg_types(self):
+return None
+
+def __call__(self, obj):
+return obj['_M_impl']['_M_finish'] - obj['_M_impl']['_M_start']
+
+class VectorSubscriptWorker(gdb.xmethod.XMethodWorker):
+def __init__(self):
+self.name = 'operator[]'
+self.enabled = True
+
+def get_arg_types(self):
+return gdb.lookup_type('std::size_t')
+
+def __call__(self, obj, subscript):
+return obj['_M_impl']['_M_start'][subscript]
+
+class VectorMethodsMatcher(gdb.xmethod.XMethodMatcher):
+def __init__(self):
+gdb.xmethod.XMethodMatcher.__init__(self,
+matcher_name_prefix + 'vector')
+self._subscript_worker = VectorSubscriptWorker()
+self._size_worker = VectorSizeWorker()
+self.methods = [self._subscript_worker, self._size_worker]
+
+def match(self, class_type, method_name):
+if not re.match('^std::vector.*$', class_type.tag):
+return None
+if method_name == 'operator[]' and self._subscript_worker.enabled:
+return self._subscript_worker
+elif method_name == 'size' and self._size_worker.enabled:
+return self._size_worker
+
+# Xmethods for std::unique_ptr
+
+class UniquePtrGetWorker(gdb.xmethod.XMethodWorker):
+def __init__(self):
+self.name = 'get'
+self.enabled = True
+
+def get_arg_types(self):
+return None
+
+def __call__(self, obj):
+return obj['_M_t']['_M_head_impl']
+
+class UniquePtrDerefWorker(UniquePtrGetWorker):
+def __init__(self):
+UniquePtrGetWorker.__init__(self)
+self.name = 'operator*'
+
+def __call__(self, obj):
+return
Re: [DOC Patch] Explicit Register Variables
On Jun 30, 2014, at 4:10 PM, David Wohlferd d...@limegreensocks.com wrote: - Vague descriptions of things that are reported to work on certain platforms are not useful (On the SPARC, there are reports that). I'd disagree. But what's more important here is the registers that are available are a function of the ABI and for someone to attempt to use this feature, they're going to have to be intimately aware of the ABI of their target. If we could say On the SPARC, these registers can be used for I'd be tempted to leave it as an example. But saying Well, someone once said they thought it might work this way on this one specific platform is not helpful for either SPARC or non-SPARC users. So, we can do s/there are reports that//; s/should be suitable, as should/are suitable, as are/ and remove the wishy washy language. If someone later wants to correct the definitive statement in some way, they are welcome to it.
