[PATCH GCC]Improving register pressure directed hoist
Hi,
This patch improves register pressure directed hoist by accurately
monitoring the change of register pressure during code hoisting and use this
information to feedback further code hoisting. To know how register pressure
is affected when hoisting an expression(x+y) through a basic block, the
patch checks:
1. whether x/y is live at the end of the basic block, the live range can't
be shrunk if yes. The live information at the IN/OUT of basic blocks have to
be updated manually by the patch, since these information won't be updated
until next call to function df_analyze.
2. whether x/y is referred by any other instructions in the basic block,
the live range can't be shrunk if yes. This is done by iterating all
references of x/y by using DF cached information and checking whether any
reference is made in this basic block.
Since GCC calls should_hoist_expr_to_dom to determine whether an expression
should be hoisted, this patch saves/restores/updates LIVE and register
pressure information before/after the call by introducing back-trace
utility.
Though additional computation is introduced by this patch, I bootstrapped
x86 with patch enabled and see no obvious slow down in compilation time.
With this patch, the effect on CSiBE of option -fira-hoist-pressure is
improved as below:
improvement
Thumb1 0.22%(0.13% before this patch)
ARM 0.16%(0.12% before)
MIPS0.25%(0.24% before)
PowerPC 0.62%
Thumb2 X
x86 X
x86_64 X
Note: Though X means no obvious effect on code size, this patch does improve
it a little bit on these targets.
I bootstrapped this patch on x86/x86_64, and tested it on
x86/x86_64/arm-none-eabi. Is it OK?
Thanks.
2012-10-29 Bin Cheng bin.ch...@arm.com
* gcse.c: (struct bb_data): Add new fields, old_pressure, live_in
and backup.
(calculate_bb_reg_pressure): Initialize live_in and backup.
(update_bb_reg_pressure): New.
(should_hoist_expr_to_dom): Add new parameter from.
Monitor the change of reg pressure and use it to drive hoisting.
(hoist_code): Update LIVE and reg pressure information.
gcc/testsuite/ChangeLog
2012-10-29 Bin Cheng bin.ch...@arm.com
* gcc.dg/hoist-register-pressure-3.c: New test.
Index: gcc/testsuite/gcc.dg/hoist-register-pressure-3.c
===
--- gcc/testsuite/gcc.dg/hoist-register-pressure-3.c(revision 0)
+++ gcc/testsuite/gcc.dg/hoist-register-pressure-3.c(revision 0)
@@ -0,0 +1,32 @@
+/* { dg-options -Os -fdump-rtl-hoist } */
+/* { dg-final { scan-rtl-dump PRE/HOIST: end of bb .* copying expression
hoist } } */
+
+#define BUF 100
+int a[BUF];
+
+void com (int);
+void bar (int);
+
+int foo (int x, int y, int z)
+{
+ /* x+y won't be hoisted if -fira-hoist-pressure is disabled,
+ because its rtx_cost is too small. */
+ if (z)
+{
+ a[1] = a[0] + a[2] + a[3] + a[4] + a[5] + a[6];
+ a[2] = a[1] + a[3] + a[5] + a[5] + a[6] + a[7];
+ a[3] = a[2] + a[5] + a[7] + a[6] + a[7] + a[8];
+ a[4] = a[3] + a[7] + a[11] + a[7] + a[8] + a[9];
+ a[5] = a[5] + a[11] + a[13] + a[8] + a[9] + a[10];
+ a[6] = a[7] + a[13] + a[17] + a[9] + a[10] + a[11];
+ a[7] = a[11] + a[17] + a[19] + a[10] + a[11] + a[12];
+ com (x+y);
+}
+ else
+{
+ bar (x+y);
+}
+
+ return 0;
+}
+
Index: gcc/gcse.c
===
--- gcc/gcse.c (revision 192758)
+++ gcc/gcse.c (working copy)
@@ -20,9 +20,6 @@ along with GCC; see the file COPYING3. If not see
/* TODO
- reordering of memory allocation and freeing to be more space efficient
- - simulate register pressure change of each basic block accurately during
- hoist process. But I doubt the benefit since most expressions hoisted
- are constant or address, which usually won't reduce register pressure.
- calc rough register pressure information and use the info to drive all
kinds of code motion (including code hoisting) in a unified way.
*/
@@ -421,6 +418,15 @@ struct bb_data
/* Maximal register pressure inside basic block for given register class
(defined only for the pressure classes). */
int max_reg_pressure[N_REG_CLASSES];
+ /* Recorded register pressure of basic block before trying to hoist
+ an expression. Will be used to restore the register pressure
+ if the expression should not be hoisted. */
+ int old_pressure;
+ /* Recorded register live_in info of basic block during code hoisting
+ process. BACKUP is used to record live_in info before trying to
+ hoist an expression, and will be used to restore LIVE_IN if the
+ expression should not be hoisted. */
+ bitmap live_in, backup;
};
#define BB_DATA(bb) ((struct bb_data *) (bb)-aux)
@@ -481,7 +487,7 @@ static void compute_code_hoist_vbeinout (void);
static void compute_code_hoist_data (void);
static int
Re: [RFC PATCH, i386]: Remove peephole2s for (subreg (operator (...)(...))) RTXes
On Sun, Oct 28, 2012 at 2:37 AM, H.J. Lu hjl.to...@gmail.com wrote:
As suggested by Richard S. [1], after the patch that converts subreg:M
(op:N (...)(...)) to op:M (subreg:M (...) subreg:M (...)), we can
remove several peephole2 patterns that handle subregs of PLUS, MINUS
and MULT operators. I have attached RFC prototype patch that will
trigger an ICE when to-be-removed pattern triggers, with the intention
that these patterns wil be removed entirely (An invalid pattern was
indeed generated elsewhere, see patch).
I have committed following version that avoids all failures, reported by H.J.:
2012-10-29 Uros Bizjak ubiz...@gmail.com
* config/i386/i386.c (ix86_decompose_address): Use simplify_gen_subreg
to generate SImode equivalent of address, zero-extended with AND RTX.
* config/i386/i386.md (ashift to lea splitter): Split to SImode mult.
(simple lea to add/shift peephole2s): Remove peephole2s that operate
on subregs of DImode operations.
Re-tested on x86_64-pc-linux-gnu {,-m32} and committed to mainline SVN.
Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 192906)
+++ config/i386/i386.c (working copy)
@@ -11821,7 +11821,11 @@ ix86_decompose_address (rtx addr, struct ix86_addr
return 0;
}
else if (GET_MODE (addr) == DImode)
- addr = gen_rtx_SUBREG (SImode, addr, 0);
+ {
+ addr = simplify_gen_subreg (SImode, addr, DImode, 0);
+ if (addr == NULL_RTX)
+ return 0;
+ }
else if (GET_MODE (addr) != VOIDmode)
return 0;
}
Index: config/i386/i386.md
===
--- config/i386/i386.md (revision 192906)
+++ config/i386/i386.md (working copy)
@@ -9600,10 +9600,10 @@
TARGET_64BIT reload_completed
true_regnum (operands[0]) != true_regnum (operands[1])
[(set (match_dup 0)
- (zero_extend:DI (subreg:SI (mult:DI (match_dup 1) (match_dup 2)) 0)))]
+ (zero_extend:DI (mult:SI (match_dup 1) (match_dup 2]
{
- operands[1] = gen_lowpart (DImode, operands[1]);
- operands[2] = gen_int_mode (1 INTVAL (operands[2]), DImode);
+ operands[1] = gen_lowpart (SImode, operands[1]);
+ operands[2] = gen_int_mode (1 INTVAL (operands[2]), SImode);
})
;; This pattern can't accept a variable shift count, since shifts by
@@ -17358,28 +17358,6 @@
(clobber (reg:CC FLAGS_REG))])])
(define_peephole2
- [(set (match_operand:SI 0 register_operand)
- (subreg:SI (plus:DI (match_operand:DI 1 register_operand)
- (match_operand:DI 2 nonmemory_operand)) 0))]
- TARGET_64BIT !TARGET_OPT_AGU
-REGNO (operands[0]) == REGNO (operands[1])
-peep2_regno_dead_p (0, FLAGS_REG)
- [(parallel [(set (match_dup 0) (plus:SI (match_dup 0) (match_dup 2)))
- (clobber (reg:CC FLAGS_REG))])]
- operands[2] = gen_lowpart (SImode, operands[2]);)
-
-(define_peephole2
- [(set (match_operand:SI 0 register_operand)
- (subreg:SI (plus:DI (match_operand:DI 1 nonmemory_operand)
- (match_operand:DI 2 register_operand)) 0))]
- TARGET_64BIT !TARGET_OPT_AGU
-REGNO (operands[0]) == REGNO (operands[2])
-peep2_regno_dead_p (0, FLAGS_REG)
- [(parallel [(set (match_dup 0) (plus:SI (match_dup 0) (match_dup 1)))
- (clobber (reg:CC FLAGS_REG))])]
- operands[1] = gen_lowpart (SImode, operands[1]);)
-
-(define_peephole2
[(set (match_operand:DI 0 register_operand)
(zero_extend:DI
(plus:SI (match_operand:SI 1 register_operand)
@@ -17404,36 +17382,6 @@
(clobber (reg:CC FLAGS_REG))])])
(define_peephole2
- [(set (match_operand:DI 0 register_operand)
- (zero_extend:DI
- (subreg:SI (plus:DI (match_dup 0)
- (match_operand:DI 1 nonmemory_operand)) 0)))]
- TARGET_64BIT !TARGET_OPT_AGU
-peep2_regno_dead_p (0, FLAGS_REG)
- [(parallel [(set (match_dup 0)
- (zero_extend:DI (plus:SI (match_dup 2) (match_dup 1
- (clobber (reg:CC FLAGS_REG))])]
-{
- operands[1] = gen_lowpart (SImode, operands[1]);
- operands[2] = gen_lowpart (SImode, operands[0]);
-})
-
-(define_peephole2
- [(set (match_operand:DI 0 register_operand)
- (zero_extend:DI
- (subreg:SI (plus:DI (match_operand:DI 1 nonmemory_operand)
- (match_dup 0)) 0)))]
- TARGET_64BIT !TARGET_OPT_AGU
-peep2_regno_dead_p (0, FLAGS_REG)
- [(parallel [(set (match_dup 0)
- (zero_extend:DI (plus:SI (match_dup 2) (match_dup 1
- (clobber (reg:CC FLAGS_REG))])]
-{
- operands[1] = gen_lowpart (SImode, operands[1]);
- operands[2] = gen_lowpart (SImode, operands[0]);
-})
-
-(define_peephole2
[(set (match_operand:SWI48 0 register_operand)
(mult:SWI48 (match_dup 0)
[PATCH/AARCH64] Fix build after __builtin_thread_pointer changes
Hi,
Right now aarch64-*-* is broken because the C front-end also defines
a __builtin_thread_pointer which causes an internal error. We don't
need to do the __builtin_thread_pointer support in the back-end. This
patch moves aarch64 to using the common code instead.
OK? Built for aarch64-thunder-elf.
Thanks,
Andrew Pinski
ChangeLog:
* config/aarch64/aarch64-protos.h (aarch64_load_tp): New proto.
* config/aarch64/aarch64.c (aarch64_load_tp): Export.
(aarch64_init_builtins): Don't add __builtin_thread_pointer builtin.
* config/aarch64/aarch64.h (aarch64_builtins): Delete
AARCH64_BUILTIN_THREAD_POINTER.
* config/aarch64/aarch64.md (get_thread_pointerdi): New pattern.
diff --git a/gcc/config/aarch64/aarch64-protos.h
b/gcc/config/aarch64/aarch64-protos.h
index 765d192..ca4e306 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -254,6 +254,7 @@ bool aarch64_split_128bit_move_p (rtx, rtx);
bool aarch64_legitimate_address_p (enum machine_mode, rtx, RTX_CODE, bool);
enum machine_mode aarch64_select_cc_mode (RTX_CODE, rtx, rtx);
rtx aarch64_gen_compare_reg (RTX_CODE, rtx, rtx);
+rtx aarch64_load_tp (rtx);
#endif /* RTX_CODE */
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1bc0c8a..aaebdf6 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -102,7 +102,6 @@ static bool aarch64_vfp_is_call_or_return_candidate (enum
machine_mode,
bool *);
static void aarch64_elf_asm_constructor (rtx, int) ATTRIBUTE_UNUSED;
static void aarch64_elf_asm_destructor (rtx, int) ATTRIBUTE_UNUSED;
-static rtx aarch64_load_tp (rtx);
static void aarch64_override_options_after_change (void);
static int aarch64_simd_valid_immediate (rtx, enum machine_mode, int, rtx *,
int *, unsigned char *, int *, int *);
@@ -4988,20 +4987,11 @@ aarch64_legitimate_constant_p (enum machine_mode mode,
rtx x)
static void
aarch64_init_builtins (void)
{
- tree ftype, decl = NULL;
-
- ftype = build_function_type (ptr_type_node, void_list_node);
- decl = add_builtin_function (__builtin_thread_pointer, ftype,
- AARCH64_BUILTIN_THREAD_POINTER, BUILT_IN_MD,
- NULL, NULL_TREE);
- TREE_NOTHROW (decl) = 1;
- TREE_READONLY (decl) = 1;
-
if (TARGET_SIMD)
init_aarch64_simd_builtins ();
}
-static rtx
+rtx
aarch64_load_tp (rtx target)
{
if (!target
@@ -5026,9 +5016,6 @@ aarch64_expand_builtin (tree exp,
tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0);
int fcode = DECL_FUNCTION_CODE (fndecl);
- if (fcode == AARCH64_BUILTIN_THREAD_POINTER)
-return aarch64_load_tp (target);
-
if (fcode = AARCH64_SIMD_BUILTIN_BASE)
return aarch64_simd_expand_builtin (fcode, exp, target);
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 3b8b033..122a7a5 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -794,7 +794,6 @@ do {
\
enum aarch64_builtins
{
AARCH64_BUILTIN_MIN,
- AARCH64_BUILTIN_THREAD_POINTER,
AARCH64_SIMD_BUILTIN_BASE
};
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index b803922..804d7e7 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -2923,6 +2923,17 @@
[(set_attr length 0)]
)
+;; Named pattern for expanding thread pointer reference.
+(define_expand get_thread_pointerdi
+ [(match_operand:DI 0 register_operand =r)]
+
+{
+ rtx tmp = aarch64_load_tp (operands[0]);
+ if (tmp != operands[0])
+emit_move_insn (operands[0], tmp);
+ DONE;
+})
+
;; AdvSIMD Stuff
(include aarch64-simd.md)
Re: Ping: [PATCH] Install error handler for out-of-memory when using STL containers
Hi Tobi, It looks straightforward to me but I am not convinced that a fortran maintainer should be giving the green light on this :-) Cheers Paul On 28 October 2012 16:28, Tobias Schlüter tobias.schlue...@physik.uni-muenchen.de wrote: Ping. This issue stands in the way of a very simple solution of PR fortran/51727. I've re-attached the patch for your convenience. On 15 Oct 2012 at 22:51:05 +0200 Tobias Schlüter wrote: The attached patch adds out-of-memory diagnostics for code using STL containers by using set_new_handler. Since the intended allocation size is not available to a new_handler, I had to forego a more detailed error message such as the one from xmalloc_failed(). fatal_error() and abort() don't give a meaningful location when the new_handler is called, so I chose to put together the error message manually as is done in xmalloc_failed(). I would have found it more appealing to have operator new call xmalloc() unless a custom allocator is given, but I don't think there's a standard way of doing this. Built and tested on the C and Fortran testsuites. Ok for trunk? Best regards, - Tobi 2012-10-15 Tobias Schlüter t...@gcc.gnu.org * toplev.c: Add '#include new'. (cxx_out_of_memory): New function. (general_init): Install cxx_out_of_memory as handler for out-of-memory condition. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [RFC PATCH, i386]: Remove peephole2s for (subreg (operator (...)(...))) RTXes
On Mon, Oct 29, 2012 at 9:07 AM, Uros Bizjak ubiz...@gmail.com wrote:
As suggested by Richard S. [1], after the patch that converts subreg:M
(op:N (...)(...)) to op:M (subreg:M (...) subreg:M (...)), we can
remove several peephole2 patterns that handle subregs of PLUS, MINUS
and MULT operators. I have attached RFC prototype patch that will
trigger an ICE when to-be-removed pattern triggers, with the intention
that these patterns wil be removed entirely (An invalid pattern was
indeed generated elsewhere, see patch).
I have committed following version that avoids all failures, reported by H.J.:
2012-10-29 Uros Bizjak ubiz...@gmail.com
* config/i386/i386.c (ix86_decompose_address): Use simplify_gen_subreg
to generate SImode equivalent of address, zero-extended with AND RTX.
* config/i386/i386.md (ashift to lea splitter): Split to SImode mult.
(simple lea to add/shift peephole2s): Remove peephole2s that operate
on subregs of DImode operations.
Re-tested on x86_64-pc-linux-gnu {,-m32} and committed to mainline SVN.
Oh, we don't have to use simplify_gen_subreg, gen_subreg is enough.
Patch committed as obvious to mainline SVN.
Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 192908)
+++ config/i386/i386.c (working copy)
@@ -11822,7 +11822,7 @@ ix86_decompose_address (rtx addr, struct ix86_addr
}
else if (GET_MODE (addr) == DImode)
{
- addr = simplify_gen_subreg (SImode, addr, DImode, 0);
+ addr = simplify_subreg (SImode, addr, DImode, 0);
if (addr == NULL_RTX)
return 0;
}
Uros.
Re: [PATCH/AARCH64] Fix build after __builtin_thread_pointer changes
On 29 Oct 2012, at 09:26, Andrew Pinski andrew.pin...@caviumnetworks.com wrote: Hi, Right now aarch64-*-* is broken because the C front-end also defines a __builtin_thread_pointer which causes an internal error. We don't need to do the __builtin_thread_pointer support in the back-end. This patch moves aarch64 to using the common code instead. OK? Built for aarch64-thunder-elf. Thanks, Andrew Pinski ChangeLog: * config/aarch64/aarch64-protos.h (aarch64_load_tp): New proto. * config/aarch64/aarch64.c (aarch64_load_tp): Export. (aarch64_init_builtins): Don't add __builtin_thread_pointer builtin. * config/aarch64/aarch64.h (aarch64_builtins): Delete AARCH64_BUILTIN_THREAD_POINTER. * config/aarch64/aarch64.md (get_thread_pointerdi): New pattern. fixaarch64.diff.txt Ok. R.
[Ada] Special case for case expression alternative moved in caller
Is_Signed_Integer_Arithmetic_Op is called to recognize an arithmetic operation on signed integers, or on if expressions and case expressions whose dependent expressions are arithmetic operation on signed integers. A special case was included for N_Case_Expression_Alternative which should have been included in the caller Apply_Arithmetic_Overflow_Minimized_Eliminated. Now done. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Yannick Moy m...@adacore.com * checks.adb (Apply_Arithmetic_Overflow_Minimized_Eliminated): Add special case for case expression alternative. (Is_Signed_Integer_Arithmetic_Op): Remove special case for case expression alternative. * exp_ch4.adb Minor reformatting. Index: checks.adb === --- checks.adb (revision 192908) +++ checks.adb (working copy) @@ -1108,8 +1108,12 @@ or else Nkind (P) in N_Membership_Test or else Nkind (P) in N_Op_Compare --- We may also be a range operand in a membership test +-- This is also true for an alternative in a case expression +or else Nkind (P) = N_Case_Expression_Alternative + +-- This is also true for a range operand in a membership test + or else (Nkind (P) = N_Range and then Nkind (Parent (P)) in N_Membership_Test) then @@ -6268,9 +6272,6 @@ when N_If_Expression | N_Case_Expression = return Is_Signed_Integer_Type (Etype (N)); - when N_Case_Expression_Alternative = -return Is_Signed_Integer_Type (Etype (Parent (N))); - when others = return False; end case; Index: exp_ch4.adb === --- exp_ch4.adb (revision 192908) +++ exp_ch4.adb (working copy) @@ -3877,8 +3877,8 @@ end if; -- Right operand is a subtype name and the subtype has a predicate. We - -- have to make sure predicate is checked, and for that we need to use - -- the standard N_In circuitry with appropriate types. + -- have to make sure the predicate is checked, and for that we need to + -- use the standard N_In circuitry with appropriate types. else pragma Assert (Present (Predicate_Function (Etype (Rop; @@ -3921,7 +3921,7 @@ -- Bnn -- end --- A bit gruesome, but here goes. +-- A bit gruesome, but there doesn't seem to be a simpler way declare Blk : constant Node_Id := Make_Bignum_Block (Loc); @@ -3937,10 +3937,8 @@ Nin := Make_In (Loc, - Left_Opnd = - Convert_To (Base_Type (Etype (Rop)), - New_Occurrence_Of (Lnn, Loc)), - Right_Opnd = New_Occurrence_Of (Etype (Rop), Loc)); + Left_Opnd = Convert_To (TB, New_Occurrence_Of (Lnn, Loc)), + Right_Opnd = New_Occurrence_Of (T, Loc)); Set_No_Minimize_Eliminate (Nin); -- Now decorate the block
[Ada] Warn for failure to parenthesize unary minus
This patch gives a warning when unary minus is used in a context that requires parentheses, and no parentheses are present, as shown by the following test: 1. function Test_Mod_B (N: in Integer) return Integer is 2. begin 3.return N mod -10; | parentheses required for unary minus 4. end Test_Mod_B; Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Robert Dewar de...@adacore.com * par-ch4.adb (P_Primary): Warn on bad use of unary minus. Index: par-ch4.adb === --- par-ch4.adb (revision 192908) +++ par-ch4.adb (working copy) @@ -2364,6 +2364,7 @@ begin -- The loop runs more than once only if misplaced pragmas are found + -- or if a misplaced unary minus is skipped. loop case Token is @@ -2537,8 +2538,15 @@ return P_Identifier; end if; +-- Minus may well be an improper attempt at a unary minus. Give +-- a message, skip the minus and keep going! + +when Tok_Minus = + Error_Msg_SC (parentheses required for unary minus); + Scan; -- past minus + -- Anything else is illegal as the first token of a primary, but --- we test for a reserved identifier so that it is treated nicely +-- we test for some common errors, to improve error messages. when others = if Is_Reserved_Identifier then
[Ada] Keep checks in Alfa mode for formal verification
Checks were suppressed to avoid undesired expansion in Alfa mode. It turns out these checks are needed during semantic analysis for the frontend to properly mark nodes as needing or not a run-time check, and the special expansion done in Alfa mode prevents the undesired expansion of checks. So reenable checks in Alfa mode. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Yannick Moy m...@adacore.com * gnat1drv.adb (Adjust_Global_Switches): Do not suppress checks in Alfa mode. Index: gnat1drv.adb === --- gnat1drv.adb(revision 192908) +++ gnat1drv.adb(working copy) @@ -419,7 +419,6 @@ -- Set switches for formal verification mode if Debug_Flag_Dot_FF then - Alfa_Mode := True; -- Set strict standard interpretation of compiler permissions @@ -448,16 +447,14 @@ Restrict.Restrictions.Set (No_Initialize_Scalars) := True; - -- Suppress all language checks since they are handled implicitly by - --the formal verification backend. - -- Turn off dynamic elaboration checks. - -- Turn off alignment checks. - -- Turn off validity checking. + -- Note: at this point we used to suppress various checks, but that + -- is not what we want. We need the semantic processing for these + -- checks (which will set flags like Do_Overflow_Check, showing the + -- points at which potential checks are required semantically). We + -- don't want the expansion associated with these checks, but that + -- happens anyway because this expansion is simply not done in the + -- Alfa version of the expander. - Suppress_Options := Suppress_All; - Dynamic_Elaboration_Checks := False; - Reset_Validity_Check_Options; - -- Kill debug of generated code, since it messes up sloc values Debug_Generated_Code := False;
[Ada] Warn for suspicious same range nested loops
If nested loops appear where the iteration ranges are over the same range of a multi-dimensional range (where the range number has been defaulted to 1), then a warning is issued as shown in the following example: 1. procedure Warn2D is 2.type E is array (Integer range , Integer range ) 3. of Integer; 4.S : Integer := 0; 5.EE : E (1 .. 3, 1 .. 3) := (Others = (Others = 0)); 6. begin 7.for J in EE'Range loop 8. for K in EE'Range loop | warning: inner range same as outer range at line 7 9. S := S + EE (J, K); 10. end loop; 11.end loop; 12. end Warn2D; Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Robert Dewar de...@adacore.com * sem_ch5.adb (Analyze_Loop_Statement): Add warning for identical inner/outer ranges. Index: sem_ch5.adb === --- sem_ch5.adb (revision 192908) +++ sem_ch5.adb (working copy) @@ -2626,6 +2626,56 @@ Push_Scope (Ent); Analyze_Iteration_Scheme (Iter); + -- Check for following case which merits a warning if the type E of is + -- a multi-dimensional array (and no explicit subscript ranges present). + + -- for J in E'Range + -- for K in E'Range + + if Present (Iter) +and then Present (Loop_Parameter_Specification (Iter)) + then + declare +LPS : constant Node_Id := Loop_Parameter_Specification (Iter); +DSD : constant Node_Id := +Original_Node (Discrete_Subtype_Definition (LPS)); + begin +if Nkind (DSD) = N_Attribute_Reference + and then Attribute_Name (DSD) = Name_Range + and then No (Expressions (DSD)) +then + declare + Typ : constant Entity_Id := Etype (Prefix (DSD)); + begin + if Is_Array_Type (Typ) +and then Number_Dimensions (Typ) 1 +and then Nkind (Parent (N)) = N_Loop_Statement +and then Present (Iteration_Scheme (Parent (N))) + then + declare +OIter : constant Node_Id := + Iteration_Scheme (Parent (N)); +OLPS : constant Node_Id := + Loop_Parameter_Specification (OIter); +ODSD : constant Node_Id := + Original_Node (Discrete_Subtype_Definition (OLPS)); + begin +if Nkind (ODSD) = N_Attribute_Reference + and then Attribute_Name (ODSD) = Name_Range + and then No (Expressions (ODSD)) + and then Etype (Prefix (ODSD)) = Typ +then + Error_Msg_Sloc := Sloc (ODSD); + Error_Msg_N + (inner range same as outer range#?, DSD); +end if; + end; + end if; + end; +end if; + end; + end if; + -- Analyze the statements of the body except in the case of an Ada 2012 -- iterator with the expander active. In this case the expander will do -- a rewrite of the loop into a while loop. We will then analyze the
[Ada] Source location of generated nodes for To_Any calls
This change modifies the source location assigned to expander generated nodes produced in the context of the distributed systems annex. Previously, we always assigned code generated for the conversion of an expression to the intermediate Any representation the location of the expression. However when such a call is generated as part of the generation of calling stubs for an RACW, this may lead to spurious ABE warnings if the RACW is declared earlier than the point where the expression occurs (case e.g. of a default value of a discriminant for a discriminated type used as formal parameter type in an RACW primitive operation, when the discriminated type declaration occurs after the RACW declaration). Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Thomas Quinot qui...@adacore.com * exp_attr.adb, exp_dist.adb, exp_dist.ads (Build_To_Any_Call): Pass an explicit Loc parameter to set the source location of generated nodes. Index: exp_attr.adb === --- exp_attr.adb(revision 192908) +++ exp_attr.adb(working copy) @@ -5141,7 +5141,8 @@ begin Rewrite (N, Build_To_Any_Call - (Convert_To (P_Type, + (Loc, + Convert_To (P_Type, Relocate_Node (First (Exprs))), Decls)); Insert_Actions (N, Decls); Analyze_And_Resolve (N, RTE (RE_Any)); Index: exp_dist.adb === --- exp_dist.adb(revision 192908) +++ exp_dist.adb(working copy) @@ -6,7 +6,7 @@ -- -- -- B o d y -- -- -- --- Copyright (C) 1992-2011, Free Software Foundation, Inc. -- +-- Copyright (C) 1992-2012, Free Software Foundation, Inc. -- -- -- -- GNAT is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -803,12 +803,14 @@ -- the declaration and entity for the newly-created function. function Build_To_Any_Call - (N : Node_Id; + (Loc : Source_Ptr; +N : Node_Id; Decls : List_Id) return Node_Id; -- Build call to To_Any attribute function with expression as actual - -- parameter. Decls is the declarations list for an appropriate - -- enclosing scope of the point where the call will be inserted; if - -- the To_Any attribute for Typ needs to be generated at this point, + -- parameter. Loc is the reference location ofr generated nodes, + -- Decls is the declarations list for an appropriate enclosing scope + -- of the point where the call will be inserted; if the To_Any + -- attribute for the type of N needs to be generated at this point, -- its declaration is appended to Decls. procedure Build_To_Any_Function @@ -879,7 +881,8 @@ renames PolyORB_Support.Helpers.Build_From_Any_Call; function Build_To_Any_Call - (N : Node_Id; + (Loc : Source_Ptr; + N : Node_Id; Decls : List_Id) return Node_Id renames PolyORB_Support.Helpers.Build_To_Any_Call; @@ -6562,7 +6565,7 @@ Object_Definition = New_Occurrence_Of (RTE (RE_Any), Loc), Expression = PolyORB_Support.Helpers.Build_To_Any_Call - (RACW_Parameter, No_List))); + (Loc, RACW_Parameter, No_List))); Statements := New_List ( Make_Procedure_Call_Statement (Loc, @@ -7362,7 +7365,7 @@ -- the first one. Expr := PolyORB_Support.Helpers.Build_To_Any_Call - (Actual_Parameter, Decls); + (Loc, Actual_Parameter, Decls); else Expr := Make_Function_Call (Loc, @@ -7448,7 +7451,7 @@ New_Occurrence_Of (RTE (RE_Any), Loc), Expression = PolyORB_Support.Helpers.Build_To_Any_Call - (Parameter_Exp, Decls))); + (Loc, Parameter_Exp, Decls))); Append_To (Extra_Formal_Statements, Add_Parameter_To_NVList (Loc, @@ -7934,7 +7937,7 @@ Parameter_Associations = New_List ( New_Occurrence_Of (Any, Loc), PolyORB_Support.Helpers.Build_To_Any_Call - (New_Occurrence_Of (Object, Loc), Decls; +
[Ada] Only one of Priority or Interrupt_Priority can be specified
ARM D.1.7 states that only one pragma priority or interrupt_priority can be given for a given entity. This patch recognizes some illegal instances of these two pragmas or aspects, that were previously undiagnosed. Compiling tasking.ads in Ada 2012 mode must yield: tasking.ads:17:07: pragma Interrupt_Priority for Monitor duplicates pragma at line 16 tasking.ads:30:07: aspect Priority for Lock previously given at line 29 --- with System; with Ada.Real_Time; use Ada.Real_Time; package Tasking is procedure Last_Chance_Handler (Msg : System.Address; Line : Integer); pragma Export (C, Last_Chance_Handler, __gnat_last_chance_handler); procedure Wait_Forever; task Test_Driver is pragma Interrupt_Priority (System.Interrupt_Priority'Last); end Test_Driver; protected Monitor is pragma Priority (System.Priority'Last); pragma Interrupt_Priority (System.Interrupt_Priority'Last); procedure Handler; pragma Attach_Handler (Handler, 12); entry Wait; private Signaled : Boolean := False; end Monitor; protected type Lock with interrupt_priority = System.priority'last, priority = System.priority'last is procedure Seize; procedure Release; private In_Use : Boolean := False; end Lock; end Tasking; Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Ed Schonberg schonb...@adacore.com * sem_aux.adb (Get_Rep_Item): Treat Priority and Interrupt_Priority as equivalent, because only one of them can be specified for a task, protected definition, or subprogram body. * aspects.adb ((Same_Aspect): The canonical aspect of Interrupt_Priority is Priority. Index: sem_aux.adb === --- sem_aux.adb (revision 192908) +++ sem_aux.adb (working copy) @@ -431,11 +431,17 @@ begin N := First_Rep_Item (E); while Present (N) loop + + -- Only one of Priority / Interrupt_Priority can be specified, so + -- return whichever one is present to catch illegal duplication. + if Nkind (N) = N_Pragma and then (Pragma_Name (N) = Nam or else (Nam = Name_Priority - and then Pragma_Name (N) = Name_Interrupt_Priority)) + and then Pragma_Name (N) = Name_Interrupt_Priority) + or else (Nam = Name_Interrupt_Priority + and then Pragma_Name (N) = Name_Priority)) then if Check_Parents then return N; Index: aspects.adb === --- aspects.adb (revision 192908) +++ aspects.adb (working copy) @@ -275,7 +275,7 @@ Aspect_Inline_Always= Aspect_Inline, Aspect_Input= Aspect_Input, Aspect_Interrupt_Handler= Aspect_Interrupt_Handler, -Aspect_Interrupt_Priority = Aspect_Interrupt_Priority, +Aspect_Interrupt_Priority = Aspect_Priority, Aspect_Invariant= Aspect_Invariant, Aspect_Iterator_Element = Aspect_Iterator_Element, Aspect_Link_Name= Aspect_Link_Name,
[Ada] Warn on redefinition of standard entities
A new warning flag -gnatw.k causes the compiler to emit a warning if a declaration redefines an entity of package Standard. Such redefinitions are usually not a good idea, since these entities are directly visible, and this can lead to confusion. This warning is off by default. The following, compiled with -gnatw.k shows the warning in action: 1. package StandNames is 2.type Integer is new Natural; | warning: redefinition of entity Integer in Standard 3.type Exceptions is (Tasking_Error, Storage_Error); | warning: redefinition of entity Tasking_Error in Standard 4. end StandNames; Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Robert Dewar de...@adacore.com * i-cstrea.ads: Avoid redefinition of standard symbol string. * prj-makr.adb: Add comment for OK redefinition of Stadard. * prj.ads: Add comment for OK redefinition of Stadard. * s-crtl.ads: Avoid redefinition of standard symbol string. * sinfo-cn.adb (Change_Identifier_To_Defining_Identifier): Generate warning for standard redefinition if Warn_On_Standard_Definition set. * usage.adb: Add lines for -gnatw.k and -gnatw.K * warnsw.adb: Set/reset Warn_On_Standard_Redefinition appropriately. * warnsw.ads (Warn_On_Standard_Redefinition): New flag. * s-stratt-xdr.adb: Avoid new warning. Index: i-cstrea.ads === --- i-cstrea.ads(revision 192908) +++ i-cstrea.ads(working copy) @@ -175,7 +175,7 @@ mode : int; size : size_t) return int; - procedure tmpnam (string : chars) renames System.CRTL.tmpnam; + procedure tmpnam (str : chars) renames System.CRTL.tmpnam; -- The parameter must be a pointer to a string buffer of at least L_tmpnam -- bytes (the call with a null parameter is not supported). The returned -- value, which is just a copy of the input argument, is discarded. Index: prj.ads === --- prj.ads (revision 192908) +++ prj.ads (working copy) @@ -68,14 +68,21 @@ type Yes_No_Unknown is (Yes, No, Unknown); -- Tri-state to decide if -lgnarl is needed when linking + pragma Warnings (Off); type Project_Qualifier is (Unspecified, + + -- The following clash with Standard is OK, and justified by the context + -- which really wants to use the same set of qualifiers. + Standard, + Library, Configuration, Dry, Aggregate, Aggregate_Library); + pragma Warnings (On); -- Qualifiers that can prefix the reserved word project in a project -- file: --Standard: standard project ... @@ -1188,8 +1195,18 @@ -- The following record describes a project file representation - type Standalone is (No, Standard, Encapsulated); + pragma Warnings (Off); + type Standalone is + (No, + -- The following clash with Standard is OK, and justified by the context + -- which really wants to use the same set of qualifiers. + + Standard, + + Encapsulated); + pragma Warnings (On); + type Project_Data (Qualifier : Project_Qualifier := Unspecified) is record - Index: prj-makr.adb === --- prj-makr.adb(revision 192908) +++ prj-makr.adb(working copy) @@ -6,7 +6,7 @@ -- -- -- B o d y -- -- -- --- Copyright (C) 2001-2011, Free Software Foundation, Inc. -- +-- Copyright (C) 2001-2012, Free Software Foundation, Inc. -- -- -- -- GNAT is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -120,7 +120,12 @@ Non_Empty_Node : constant Project_Node_Id := 1; -- Used for the With_Clause of the naming project + -- Turn off warnings for now around this redefinition of True and False, + -- but it really seems a bit horrible to do this redefinition ??? + + pragma Warnings (Off); type Matched_Type is (True, False, Excluded); + pragma Warnings (On); Naming_File_Suffix : constant String := _naming; Source_List_File_Suffix : constant String := _source_list.txt; Index: s-crtl.ads === --- s-crtl.ads (revision 192908) +++ s-crtl.ads (working copy) @@ -177,7 +177,7 @@ size : size_t) return int; pragma Import (C, setvbuf, setvbuf); - procedure
[Ada] Internal bindgen.adb cleanup
This patch remove a duplicated logic to check wether a package is in the closure. No functional change. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Tristan Gingold ging...@adacore.com * bindgen.adb (Check_File_In_Partition, Check_System_Restrictions_Used): Removed. (Check_Dispatching_Domains_Used): Removed. (Gen_Adafinal): Remove call to above procedures. (Resolve_Binder_Options): Handle system restrictions and dispatching domains. Index: bindgen.adb === --- bindgen.adb (revision 192918) +++ bindgen.adb (working copy) @@ -63,20 +63,20 @@ Num_Elab_Calls : Nat := 0; -- Number of generated calls to elaboration routines - System_Restrictions_Used : Boolean; + System_Restrictions_Used : Boolean := False; -- Flag indicating whether the unit System.Restrictions is in the closure - -- of the partition. This is set by Check_System_Restrictions_Used, and + -- of the partition. This is set by Resolve_Binder_Options, and -- is used to determine whether or not to initialize the restrictions -- information in the body of the binder generated file (we do not want -- to do this unconditionally, since it drags in the System.Restrictions -- unit unconditionally, which is unpleasand, especially for ZFP etc.) - Dispatching_Domains_Used : Boolean; + Dispatching_Domains_Used : Boolean := False; -- Flag indicating whether multiprocessor dispatching domains are used in - -- the closure of the partition. This is set by - -- Check_Dispatching_Domains_Used, and is used to call the routine to - -- disallow the creation of new dispatching domains just before calling - -- the main procedure from the environment task. + -- the closure of the partition. This is set by Resolve_Binder_Options, + -- and is used to call the routine to disallow the creation of new + -- dispatching domains just before calling the main procedure from the + -- environment task. System_Tasking_Restricted_Stages_Used : Boolean := False; -- Flag indicating whether the unit System.Tasking.Restricted.Stages is in @@ -242,21 +242,6 @@ -- Local Subprograms -- --- - procedure Check_File_In_Partition - (File_Name : String; - Flag : out Boolean); - -- If the file indicated by File_Name is in the partition the Flag is set - -- to True, False otherwise. - - procedure Check_System_Restrictions_Used; - -- Sets flag System_Restrictions_Used (Set to True if and only if the unit - -- System.Restrictions is present in the partition, otherwise False). - - procedure Check_Dispatching_Domains_Used; - -- Sets flag Dispatching_Domains_Used to True when using the unit - -- System.Multiprocessors.Dispatching_Domains is present in the partition, - -- otherwise set to False. - procedure Gen_Adainit; -- Generates the Adainit procedure @@ -391,43 +376,6 @@ -- First writes its argument (using Set_String (S)), then writes out the -- contents of statement buffer up to Last, and reset Last to 0 - - -- Check_Dispatching_Domains_Used -- - - - procedure Check_Dispatching_Domains_Used is - begin - Check_File_In_Partition (s-mudido.ads, Dispatching_Domains_Used); - end Check_Dispatching_Domains_Used; - - - - -- Check_File_In_Partition -- - - - - procedure Check_File_In_Partition - (File_Name : String; - Flag : out Boolean) - is - begin - for J in Units.First .. Units.Last loop - if Get_Name_String (Units.Table (J).Sfile) = File_Name then -Flag := True; -return; - end if; - end loop; - - Flag := False; - end Check_File_In_Partition; - - - -- Check_System_Restrictions_Used -- - - - procedure Check_System_Restrictions_Used is - begin - Check_File_In_Partition (s-restri.ads, System_Restrictions_Used); - end Check_System_Restrictions_Used; - -- -- Gen_Adafinal -- -- @@ -2124,9 +2072,6 @@ -- Generate output file in appropriate language - Check_System_Restrictions_Used; - Check_Dispatching_Domains_Used; - Gen_Output_File_Ada (Filename); end Gen_Output_File; @@ -2869,6 +2814,23 @@ procedure Resolve_Binder_Options is + procedure Check_Package (Var : in out Boolean; Name : String); + -- Set Var to true iff the current identifier in Namet is Name. + -- Do nothing if it doesn't match. This procedure is just an helper + -- to avoid to explicitely deal with length. + + --- + -- Check_Package -- +
[Ada] Add support for returning binary message digest
If a binary digest is needed it is a waste of time to recompute it from the string encoded digest. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Pascal Obry o...@adacore.com * g-sechas.adb, g-sechas.ads: (Binary_Message_Digest): New subtype. (Digest): New versions returning a Binary_Message_Digest. (Wide_Digest): Likewise. Index: g-sechas.adb === --- g-sechas.adb(revision 192920) +++ g-sechas.adb(working copy) @@ -184,6 +184,30 @@ return Digest (C); end Digest; + function Digest (C : Context) return Binary_Message_Digest is + Hash_Bits : Stream_Element_Array + (1 .. Stream_Element_Offset (Hash_Length)); + begin + Final (C, Hash_Bits); + return Hash_Bits; + end Digest; + + function Digest (S : String) return Binary_Message_Digest is + C : Context; + begin + Update (C, S); + return Digest (C); + end Digest; + + function Digest +(A : Stream_Element_Array) return Binary_Message_Digest + is + C : Context; + begin + Update (C, A); + return Digest (C); + end Digest; + --- -- Final -- --- @@ -325,6 +349,13 @@ return Digest (C); end Wide_Digest; + function Wide_Digest (W : Wide_String) return Binary_Message_Digest is + C : Context; + begin + Wide_Update (C, W); + return Digest (C); + end Wide_Digest; + end H; - Index: g-sechas.ads === --- g-sechas.ads(revision 192920) +++ g-sechas.ads(working copy) @@ -156,6 +156,22 @@ Word_Length : constant Natural := Hash_State.Word'Size / 8; Hash_Length : constant Natural := Hash_Words * Word_Length; + subtype Binary_Message_Digest is +Stream_Element_Array (1 .. Stream_Element_Offset (Hash_Length)); + -- The fixed-length byte array returned by Digest, providing + -- the hash in binary representation. + + function Digest (C : Context) return Binary_Message_Digest; + -- Return hash for the data accumulated with C + + function Digest (S : String) return Binary_Message_Digest; + function Wide_Digest (W : Wide_String) return Binary_Message_Digest; + function Digest +(A : Stream_Element_Array) return Binary_Message_Digest; + -- These functions are equivalent to the corresponding Update (or + -- Wide_Update) on a default initialized Context, followed by Digest + -- on the resulting Context. + subtype Message_Digest is String (1 .. 2 * Hash_Length); -- The fixed-length string returned by Digest, providing the hash in -- hexadecimal representation.
Re: [patch][rfc] Clean up CFG dumping
On Mon, Jul 16, 2012 at 8:57 AM, Steven Bosscher stevenb@gmail.com wrote: Hello, There are comments in basic-block.h that advise to update certain parts of the compiler if a new edge flag or basic block flag is added: -/* Always update the table in cfg.c dump_edge_info. */ and - Always update the table in cfg.c dump_bb_info. */ Apparently this is not enough, because there are more places where the BB flags are dumped. For instance, cfg.c:dump_cfg_bb_info does not handle BB_MODIFIED, BB_VISITED, and BB_IN_TRANSACTION, and dump_bb_info doesn't even exist in cfg.c (it's in cfgrtl.c). The flags also aren't documented very well in the code. Furthermore, there are multiple places where common (i.e. IR agnostic) basic block information is dumped, with some functions taking TDF_* flags and others not, some functions taking a FILE as the first argument and others as the second argument, etc. In short: Unnecessarily messy. The attached patch cleans up the worst of it: * A new file cfg-flags.def is used to define the BB_* and EDGE_* flags. The names are the full string of the name of the flag in uppercase, that's a change from before. I can add a separate name field to DEF_EDGE_FLAG and DEF_BB_FLAG if necessary. * Now that there is dumpfile.h for the TDF_* masks, it's easier to use them everywhere. I have added one new flag to dump with the ;; prefix (I think it should be used in more places, but that's something for later, perhaps). * All basic block header/footer and edge dumping is consolidated in dump_edge_info and dump_bb_info. This affects GIMPLE dump the most, because it uses a different form of dumping for basic block predecessors and successors. I expect some fall-out in the test suite (patterns that no longer match) that I'll have to address before the patch is ready for mainline. * Slim RTL printing is better integrated: print_rtl_with_bb takes flags and uses dump_rtl_slim if TDF_SLIM is passed. The same happens in rtl_dump_bb, which always dumps non-slim RTL without this patch. Bootstrapped on powerpc64-unknown-linux-gnu. Testing will probably reveal some test suite pattern mismatches, and I also still want to bootstraptest this on x86_64. I'd like to hear what people think of the cfg-flags.def change. Ciao! Steven * dumpfile.h (TDF_COMMENT): New define. * basic-block.h (EDGE_FALLTHRU, EDGE_ABNORMAL, EDGE_ABNORMAL_CALL, EDGE_EH, EDGE_FAKE, EDGE_DFS_BACK, EDGE_CAN_FALLTHRU, EDGE_IRREDUCIBLE_LOOP, EDGE_SIBCALL, EDGE_LOOP_EXIT, EDGE_TRUE_VALUE, EDGE_FALSE_VALUE, EDGE_EXECUTABLE, EDGE_CROSSING, EDGE_PRESERVE): Move to new file cfg-flags.h. (enum cfg_edge_flags): New enum, using cfg-flags.h. (EDGE_ALL_FLAGS): Compute value automatically. (BB_NEW, BB_REACHABLE, BB_IRREDUCIBLE_LOOP, BB_SUPERBLOCK, BB_DISABLE_SCHEDULE, BB_HOT_PARTITION, BB_COLD_PARTITION, BB_DUPLICATED, BB_NON_LOCAL_GOTO_TARGET, BB_RTL, BB_FORWARDER_BLOCK, BB_NONTHREADABLE_BLOCK, BB_MODIFIED, BB_VISITED, BB_IN_TRANSACTION): Move to new file cfg-flags.h. (enum bb_flags): Rename to cfg_bb_flags. Use cfg-flags.h. (BB_ALL_FLAGS): New, compute value automatically. (dump_bb_info): Update prototype. (dump_edge_info): Update prototype. * cfg-flags.h: New file. * cfg.c (dump_edge_info): Take flags argument. Be verbose only if TDF_DETAILS and not TDF_SLIM. Include cfg-flags.h for bitnames. Check that the edge flags are within the range of EDGE_ALL_FLAGS. (debug_bb): Update dump_bb call. (dump_cfg_bb_info): Remove. (dump_bb_info): New function. Use cfg-flags.h for bitnames. Adjust verbosity using TDF_* flags. Check that the basic block flags are within the range of BB_ALL_FLAGS. (brief_dump_cfg): Use dump_bb_info instead of dump_cfg_bb_info. * cfghooks.h (struct cfghooks): Update dump_bb hook, take a FILE first for consistency with other dump functions. (dump_bb): Update prototype accordingly. * cfghooks.c: Include dumpfile.h. (verify_flow_info): Update dump_edge_info calls. (dump_bb): Take a flags argument and pass it around. Use dump_bb_info to dump common information about a basic block. (dump_flow_info): Moved here from cfgrtl.c. Make IL agnostic. (debug_flow_info): Moved here from cfgrtl.c. * profile.c (is_edge_inconsistent): Update dump_bb calls. * loop-invariant.c (find_defs): Update print_rtl_with_bb call. * rtl.h (debug_bb_n_slim, debug_bb_slim, print_rtl_slim, print_rtl_slim_with_bb): Remove prototypes. (dump_insn_slim): Adjust prototype to take a const_rtx. (print_rtl_with_bb): Adjust prototype. * sched-rgn.c (debug_region): Use dump_bb instead of debug_bb_n_slim. * sched-vis.c
[Ada] Add a new pragma Assert_And_Cut for formal verification tools
The new pragma Assert_And_Cut behaves exactly like pragma Assert for
compilation, but it informs formal verification tools of program points where
the tool may simpllify it treatment by using the given assertion.
Tested on x86_64-pc-linux-gnu, committed on trunk
2012-10-29 Yannick Moy m...@adacore.com
* gnat_rm.texi: Describe new pragma Assert_And_Cut.
* par-prag.adb, sem_prag.adb, snames.ads-tmpl: Add new pragma
and treat it like pragma Assert.
Index: gnat_rm.texi
===
--- gnat_rm.texi(revision 192918)
+++ gnat_rm.texi(working copy)
@@ -105,6 +105,7 @@
* Pragma Ada_2012::
* Pragma Annotate::
* Pragma Assert::
+* Pragma Assert_And_Cut::
* Pragma Assertion_Policy::
* Pragma Assume_No_Invalid_Values::
* Pragma Ast_Entry::
@@ -843,6 +844,7 @@
* Pragma Ada_2012::
* Pragma Annotate::
* Pragma Assert::
+* Pragma Assert_And_Cut::
* Pragma Assertion_Policy::
* Pragma Assume_No_Invalid_Values::
* Pragma Ast_Entry::
@@ -1195,7 +1197,23 @@
of Ada, and the DISABLE policy is an implementation-defined
addition.
+@node Pragma Assert_And_Cut
+@unnumberedsec Pragma Assert_And_Cut
+@findex Assert_And_Cut
+@noindent
+Syntax:
+@smallexample @c ada
+pragma Assert_And_Cut (
+ boolean_EXPRESSION
+ [, string_EXPRESSION]);
+@end smallexample
+@noindent
+The effect of this pragma for compilation is exactly the same as the one
+of pragma @code{Assert}. This pragma is used to help formal verification
+tools by marking program points where the tool can simplify precise
+knowledge about execution based on the assertion given.
+
@node Pragma Assertion_Policy
@unnumberedsec Pragma Assertion_Policy
@findex Debug_Policy
Index: sem_prag.adb
===
--- sem_prag.adb(revision 192918)
+++ sem_prag.adb(working copy)
@@ -6759,14 +6759,17 @@
end if;
end Annotate;
-
- -- Assert --
-
+ -
+ -- Assert Assert_And_Cut --
+ -
-- pragma Assert ([Check =] Boolean_EXPRESSION
-- [, [Message =] Static_String_EXPRESSION]);
- when Pragma_Assert = Assert : declare
+ -- pragma Assert_And_Cut ([Check =] Boolean_EXPRESSION
+ -- [, [Message =] Static_String_EXPRESSION]);
+
+ when Pragma_Assert | Pragma_Assert_And_Cut = Assert : declare
Expr : Node_Id;
Newa : List_Id;
@@ -6784,6 +6787,13 @@
-- So rewrite pragma in this manner, transfer the message
-- argument if present, and analyze the result
+-- Pragma Assert_And_Cut is treated exactly like pragma Assert by
+-- the frontend. Formal verification tools may use it to cut the
+-- paths through the code, to make verification tractable. When
+-- dealing with a semantically analyzed tree, the information that
+-- a Check node N corresponds to a source Assert_And_Cut pragma
+-- can be retrieved from the pragma kind of Original_Node(N).
+
Expr := Get_Pragma_Arg (Arg1);
Newa := New_List (
Make_Pragma_Argument_Association (Loc,
@@ -15185,6 +15195,7 @@
Pragma_All_Calls_Remote = -1,
Pragma_Annotate = -1,
Pragma_Assert = -1,
+ Pragma_Assert_And_Cut = -1,
Pragma_Assertion_Policy = 0,
Pragma_Assume_No_Invalid_Values = 0,
Pragma_Asynchronous = -1,
Index: par-prag.adb
===
--- par-prag.adb(revision 192918)
+++ par-prag.adb(working copy)
@@ -1098,6 +1098,7 @@
Pragma_All_Calls_Remote |
Pragma_Annotate |
Pragma_Assert |
+ Pragma_Assert_And_Cut |
Pragma_Asynchronous |
Pragma_Atomic |
Pragma_Atomic_Components |
Index: snames.ads-tmpl
===
--- snames.ads-tmpl (revision 192918)
+++ snames.ads-tmpl (working copy)
@@ -448,6 +448,7 @@
-- correctly recognize and process Name_AST_Entry.
Name_Assert : constant Name_Id := N + $; -- Ada 05
+ Name_Assert_And_Cut : constant Name_Id := N + $; -- GNAT
Name_Asynchronous : constant Name_Id := N + $;
Name_Atomic : constant Name_Id := N + $;
Name_Atomic_Components : constant Name_Id := N + $;
@@ -1697,6 +1698,7 @@
Pragma_Abort_Defer,
[Ada] Clean up of gnatbind output (for non-standard run times)
A declaration for Is_Elaborated was emitted but never referenced. No functional change. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Tristan Gingold ging...@adacore.com * bindgen.adb (Gen_Output_File_Ada): Do not emit declaration for Is_Elaborated if not referenced. Index: bindgen.adb === --- bindgen.adb (revision 192922) +++ bindgen.adb (working copy) @@ -2394,8 +2394,13 @@ -- The B.1 (39) implementation advice says that the adainit/adafinal -- routines should be idempotent. Generate a flag to ensure that. + -- This is not needed if we are suppressing the standard library + -- since it would never be referenced. - WBI ( Is_Elaborated : Boolean := False;); + if not Suppress_Standard_Library_On_Target then +WBI ( Is_Elaborated : Boolean := False;); + end if; + WBI (); end if;
[Ada] Duplicate entries for parameters in gnatxref output
When a call to a subprogram in another package uses named parameters, gnatxref will sometimes output duplicate entries for the parameter, depending on the order in which the ALI files are read. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Emmanuel Briot br...@adacore.com * xr_tabls.adb, xr_tabls.ads (Add_Declaration, Add_Reference): No longer assume that a parameter declaration is seen after the subprogram that uses it. Index: xr_tabls.adb === --- xr_tabls.adb(revision 192918) +++ xr_tabls.adb(working copy) @@ -6,7 +6,7 @@ -- -- -- B o d y -- -- -- --- Copyright (C) 1998-2010, Free Software Foundation, Inc. -- +-- Copyright (C) 1998-2012, Free Software Foundation, Inc. -- -- -- -- GNAT is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -223,6 +223,7 @@ Line : Natural; Column : Natural; Decl_Type: Character; + Is_Parameter : Boolean := False; Remove_Only : Boolean := False; Symbol_Match : Boolean := True) return Declaration_Reference @@ -235,7 +236,7 @@ New_Decl : Declaration_Reference := Entities_HTable.Get (Key'Unchecked_Access); - Is_Parameter : Boolean := False; + Is_Param : Boolean := Is_Parameter; begin -- Insert the Declaration in the table. There might already be a @@ -243,7 +244,7 @@ -- need to check that first. if New_Decl /= null and then New_Decl.Symbol_Length = 0 then - Is_Parameter := New_Decl.Is_Parameter; + Is_Param := Is_Parameter or else New_Decl.Is_Parameter; Entities_HTable.Remove (Key'Unrestricted_Access); Entities_Count := Entities_Count - 1; Free (New_Decl.Key); @@ -269,7 +270,7 @@ Column= Column, Source_Line = null, Next = null), - Is_Parameter = Is_Parameter, + Is_Parameter = Is_Param, Decl_Type = Decl_Type, Body_Ref = null, Ref_Ref = null, @@ -294,6 +295,10 @@ then New_Decl.Match := Default_Match or else Match (File_Ref, Line, Column); + New_Decl.Is_Parameter := New_Decl.Is_Parameter or else Is_Param; + + elsif New_Decl /= null then + New_Decl.Is_Parameter := New_Decl.Is_Parameter or else Is_Param; end if; return New_Decl; @@ -392,6 +397,8 @@ Labels_As_Ref : Boolean) is New_Ref : Reference; + New_Decl : Declaration_Reference; + pragma Unreferenced (New_Decl); begin case Ref_Type is @@ -407,37 +414,22 @@ when '=' | '' | '' | '^' = -- Create a dummy declaration in the table to report it as a --- parameter. Note that the current declaration for the subprogram --- comes before the declaration of the parameter. +-- parameter. +-- In a given ALI file, the declaration of the subprogram comes +-- before the declaration of the parameter. However, it is +-- possible that another ALI file has been parsed that also +-- references the parameter (for instance a named parameter in a +-- call), so we need to check whether there already exists a +-- declaration for the parameter. -declare - Key : constant String := -Key_From_Ref (File_Ref, Line, Column); - New_Decl : Declaration_Reference; +New_Decl := Add_Declaration + (File_Ref = File_Ref, + Symbol= , + Line = Line, + Column= Column, + Decl_Type = ' ', + Is_Parameter = True); -begin - New_Decl := new Declaration_Record' - (Symbol_Length = 0, - Symbol= , - Key = new String'(Key), - Decl = new Reference_Record' - (File = File_Ref, - Line = Line, - Column= Column, - Source_Line = null, - Next = null), - Is_Parameter =
[Ada] Implement Pragma Partition_Elaboration_Policy
Initial work to implement pragma Partition_Elaboration_Policy. Currently, only consistency is checked, the runtime only implements one policy. A following patch will add a pragma Partition_Elaboration_Policy in the runtime to enforce the policy (when tasking is used). Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Tristan Gingold ging...@adacore.com * lib-writ.adb (Write_ALI): Emit partition elaboration policy in P line. * lib-writ.ads: Document partition elaboration policy indication. * sem_prag.adb (Check_Arg_Is_Partition_Elaboration_Policy): New procedure. (Analyze_Pragma): Handle Partition_Elaboration_Policy. (Sig_Flags): Add flag for Pragma_Partition_Elaboration_Policy * ali.adb (Initialize_ALI): Init Partition_Elaboration_Policy_Specified. (Scan_ALI): Read Ex indications. * ali.ads: ALIs_Record: Add Partition_Elaboration_Policy. * par-prag.adb (Prag): Add Partition_Elaboration_Policy. * snames.adb-tmpl (Is_Partition_Elaboration_Policy_Name): New function. * opt.ads (Partition_Elaboration_Policy): Declare. (Partition_Elaboration_Policy_Sloc): Declare. * bcheck.adb (Check_Consistent_Partition_Elaboration_Policy): New procedure. (Check_Configuration_Consistency): Check partition elaboration policy consistency. * snames.ads-tmpl (Name_Partition_Elaboration_Policy): New name. (First_Partition_Elaboration_Policy_Name, Name_Concurrent, Name_Sequential, Last_Partition_Elaboration_Policy_Name): Likewise. (Pragma_Partition_Elaboration_Policy): New literal. (Is_Partition_Elaboration_Policy_Name): New function. Index: lib-writ.adb === --- lib-writ.adb(revision 192918) +++ lib-writ.adb(working copy) @@ -1099,6 +1099,11 @@ end if; end if; + if Partition_Elaboration_Policy /= ' ' then + Write_Info_Str ( E); + Write_Info_Char (Partition_Elaboration_Policy); + end if; + if not Object then Write_Info_Str ( NO); end if; Index: lib-writ.ads === --- lib-writ.ads(revision 192918) +++ lib-writ.ads(working copy) @@ -196,6 +196,10 @@ -- DB Detect_Blocking pragma is in effect for all units in this -- file. -- + -- Ex A valid Partition_Elaboration_Policy pragma applies to all + -- the units in this file, where x is the first character + -- (upper case) of the policy name (e.g. 'C' for Concurrent). + -- -- FD Configuration pragmas apply to all the units in this file -- specifying a possibly non-standard floating point format -- (VAX float with Long_Float using D_Float). Index: sem_prag.adb === --- sem_prag.adb(revision 192926) +++ sem_prag.adb(working copy) @@ -505,6 +505,10 @@ -- Check the specified argument Arg to make sure that it is a valid -- locking policy name. If not give error and raise Pragma_Exit. + procedure Check_Arg_Is_Partition_Elaboration_Policy (Arg : Node_Id); + -- Check the specified argument Arg to make sure that it is a valid + -- elaboration policy name. If not give error and raise Pragma_Exit. + procedure Check_Arg_Is_One_Of (Arg: Node_Id; N1, N2 : Name_Id); @@ -1190,6 +1194,22 @@ end if; end Check_Arg_Is_Locking_Policy; + --- + -- Check_Arg_Is_Partition_Elaboration_Policy -- + --- + + procedure Check_Arg_Is_Partition_Elaboration_Policy (Arg : Node_Id) is + Argx : constant Node_Id := Get_Pragma_Arg (Arg); + + begin + Check_Arg_Is_Identifier (Argx); + + if not Is_Partition_Elaboration_Policy_Name (Chars (Argx)) then +Error_Pragma_Arg + ( is not a valid partition elaboration policy name, Argx); + end if; + end Check_Arg_Is_Partition_Elaboration_Policy; + - -- Check_Arg_Is_One_Of -- - @@ -12039,6 +12059,53 @@ when Pragma_Page = null; + -- + -- Partition_Elaboration_Policy -- + -- + + -- pragma Partition_Elaboration_Policy (policy_IDENTIFIER); + + when Pragma_Partition_Elaboration_Policy = declare +subtype PEP_Range is Name_Id + range First_Partition_Elaboration_Policy_Name + .. Last_Partition_Elaboration_Policy_Name; +PEP_Val : PEP_Range; +PEP :
Re: [RFC PATCH, i386]: Remove peephole2s for (subreg (operator (...)(...))) RTXes
On Mon, Oct 29, 2012 at 10:06 AM, Uros Bizjak ubiz...@gmail.com wrote:
As suggested by Richard S. [1], after the patch that converts subreg:M
(op:N (...)(...)) to op:M (subreg:M (...) subreg:M (...)), we can
remove several peephole2 patterns that handle subregs of PLUS, MINUS
and MULT operators. I have attached RFC prototype patch that will
trigger an ICE when to-be-removed pattern triggers, with the intention
that these patterns wil be removed entirely (An invalid pattern was
indeed generated elsewhere, see patch).
I have committed following version that avoids all failures, reported by
H.J.:
2012-10-29 Uros Bizjak ubiz...@gmail.com
* config/i386/i386.c (ix86_decompose_address): Use
simplify_gen_subreg
to generate SImode equivalent of address, zero-extended with AND RTX.
* config/i386/i386.md (ashift to lea splitter): Split to SImode mult.
(simple lea to add/shift peephole2s): Remove peephole2s that operate
on subregs of DImode operations.
Re-tested on x86_64-pc-linux-gnu {,-m32} and committed to mainline SVN.
Oh, we don't have to use simplify_gen_subreg, gen_subreg is enough.
Not really, then we won't generate SUBREGs of pseudos...
Reverting the commit.
Uros.
[Ada] Static entry [family] names for VMS Debug
This patch reimplements the mechanism which creates string names for entries and families. The names are now created statically during object initialization and associated with the concurrent object's Protection_Entries or ATCB. This approach eliminates the need for runtime support (allocation, deallocation) of the names. No simple test as this requires VMS and gdb to demonstrate Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Hristian Kirtchev kirtc...@adacore.com * exp_ch3.adb (Build_Initialization_Call): Create static strings which denote entry [family] names and associate them with the object's Protection_Entries or ATCB. (Build_Init_Statements): Remove local variable Names. Do not generate the entry [family] names inside the init proc because they are now static. * exp_ch9.adb (Build_Entry_Names): Reimplemented. The strings which denote entry [family] names are now generated statically and associated with the concurrent object's Protection_Entries or ATCB during initialization. * exp_ch9.ads (Build_Entry_Names): Change subprogram profile and associated comment on usage. * rtsfind.ads: Add the following entries to tables RE_Id and RE_Unit_Table: RE_Protected_Entry_Names_Array RE_Task_Entry_Names_Array RO_PE_Number_Of_Entries RO_PE_Set_Entry_Names RO_ST_Number_Of_Entries RO_ST_Set_Entry_Names Remove the following entries from tables RE_Id and RE_Unit_Table: RO_PE_Set_Entry_Name RO_TS_Set_Entry_Name * s-taskin.adb: Remove with clause for Ada.Unchecked_Deallocation. (Free_Entry_Names_Array): Removed. (Number_Of_Entries): New routine. (Set_Entry_Names): New routine. * s-taskin.ads: Rename type Entry_Names_Array to Task_Entry_Names_Array. Rename type Entry_Names_Array_Access to Task_Entry_Names_Access. Update the type of ACTB field Entry_Names and add a comment on its protection status. (Free_Entry_Names_Array): Removed. (Number_Of_Entries): New routine. (Set_Entry_Names): New routine. * s-tassta.adb (Create_Task): Remove formal parameter Build_Entry_Names. Do not allocate an array to hold the string names of entries and families. (Free_Entry_Names): Removed. (Free_Task): Remove the call to Free_Entry_Names. (Set_Entry_Name): Removed. (Vulnerable_Free_Task): Remove the call to Free_Entry_Names. * s-tassta.ads (Create_Task): Remove formal parameter Build_Entry_Names along with associated comment. (Set_Entry_Name): Removed. * s-tpoben.adb: Remove with clause for Ada.Unchecked_Deallocation. (Finalize): Remove the call to Free_Entry_Names. (Free_Entry_Names): Removed. (Initialize_Protection_Entries): Remove formal parameter Build_Entry_Names. Do not allocate an array to hold the string names of entries and families. (Number_Of_Entries): New routine. (Set_Entry_Name): Removed. (Set_Entry_Names): New routine. * s-tpoben.ads: Add types Protected_Entry_Names_Array and Protected_Entry_Names_Access. Update the type of Protection_Enties field Entry_Names. (Initialize_Protection_Entries): Remove formal parameter Build_Entry_Names along with associated comment. (Number_Of_Entries): New routine. (Set_Entry_Name): Removed. (Set_Entry_Names): New routine. Index: exp_ch9.adb === --- exp_ch9.adb (revision 192929) +++ exp_ch9.adb (working copy) @@ -1363,59 +1363,54 @@ -- Build_Entry_Names -- --- - function Build_Entry_Names (Conc_Typ : Entity_Id) return Node_Id is - Loc : constant Source_Ptr := Sloc (Conc_Typ); - B_Decls : List_Id; - B_Stmts : List_Id; - Comp : Node_Id; - Index : Entity_Id; - Index_Typ : RE_Id; - Typ : Entity_Id := Conc_Typ; + procedure Build_Entry_Names + (Obj_Ref : Node_Id; + Obj_Typ : Entity_Id; + Stmts : List_Id) + is + Loc : constant Source_Ptr := Sloc (Obj_Ref); + Data : Entity_Id := Empty; + Index : Entity_Id := Empty; + Typ : Entity_Id := Obj_Typ; - procedure Build_Entry_Family_Name (Id : Entity_Id); - -- Generate: - --for Lnn in Family_Low .. Family_High loop - -- Inn := Inn + 1; - -- Set_Entry_Name - -- (_init._object or _init._task_id, - -- Inn, - -- new String (Entry name( Lnn'Img ))); - --end loop; - -- Note that the bounds of the range may reference discriminants. The - -- above construct is added directly to the statements of the block. + procedure Build_Entry_Name (Comp_Id : Entity_Id); + -- Given an entry
[Ada] New implementation-defined pragma: Attribute_Definition
This change introduces a new implementation defined pragma Attribute_Definition, which allows an attribute definition clause to be expressed in a backward-compatible manner: compilers not supporting the pragma, or the specified attribute, will just ignore it. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Thomas Quinot qui...@adacore.com * gnat_rm.texi, sem_prag.adb, sem_util.adb, sem_util.ads, par-prag.adb, par-util.adb, snames.ads-tmpl (Sem_Prag.Analyze_Pragma): Handle new pragma Attribute_Definition. (Sem_Util.Bad_Attribute): New routine, moved here from par-util, so that it can be used by the above. (Par_Util.Signal_Bad_Attribute): Processing moved to Sem_Util.Bad_Attribute. Index: gnat_rm.texi === --- gnat_rm.texi(revision 192933) +++ gnat_rm.texi(working copy) @@ -107,6 +107,7 @@ * Pragma Assert:: * Pragma Assertion_Policy:: * Pragma Assume_No_Invalid_Values:: +* Pragma Attribute_Definition:: * Pragma Ast_Entry:: * Pragma C_Pass_By_Copy:: * Pragma Check:: @@ -845,6 +846,7 @@ * Pragma Assert:: * Pragma Assertion_Policy:: * Pragma Assume_No_Invalid_Values:: +* Pragma Attribute_Definition:: * Pragma Ast_Entry:: * Pragma C_Pass_By_Copy:: * Pragma Check:: @@ -1308,6 +1310,28 @@ normal use of the entry. For further details on this pragma, see the DEC Ada Language Reference Manual, section 9.12a. +@node Pragma Attribute_Definition +@unnumberedsec Pragma Attribute_Definition +@findex Attribute_Definition +@noindent +Syntax: +@smallexample @c ada +pragma Attribute_Definition + ([Attribute =] ATTRIBUTE_DESIGNATOR, + [Entity =] LOCAL_NAME, + [Expression =] EXPRESSION | NAME); +@end smallexample + +@noindent +If Attribute is a known attribute name, this pragma is equivalent to +the attribute definition clause: +@smallexample @c ada + for Entity'Attribute use Expression; +@end smallexample +else the pragma is ignored, and a warning is emitted. This allows source +code to be written that takes advantage of some new attribute, while remaining +compilable with earlier compilers. + @node Pragma C_Pass_By_Copy @unnumberedsec Pragma C_Pass_By_Copy @cindex Passing by copy Index: sem_prag.adb === --- sem_prag.adb(revision 192934) +++ sem_prag.adb(working copy) @@ -6919,6 +6919,47 @@ Assume_No_Invalid_Values := False; end if; + -- + -- Attribute_Definition -- + -- + + -- pragma Attribute_Definition + --([Attribute =] ATTRIBUTE_DESIGNATOR, + -- [Entity =] LOCAL_NAME, + -- [Expression =] EXPRESSION | NAME); + + when Pragma_Attribute_Definition = Attribute_Definition : declare +Attribute_Designator : constant Node_Id := Get_Pragma_Arg (Arg1); +Aname : Name_Id; + + begin +GNAT_Pragma; +Check_Arg_Count (3); +Check_Optional_Identifier (Arg1, attribute); +Check_Optional_Identifier (Arg2, entity); +Check_Optional_Identifier (Arg3, expression); + +if Nkind (Attribute_Designator) /= N_Identifier then + Error_Msg_N (attribute name expected, Attribute_Designator); + return; +end if; + +Check_Arg_Is_Local_Name (Arg2); + +Aname := Chars (Attribute_Designator); +if not Is_Attribute_Name (Aname) then + Bad_Attribute (Attribute_Designator, Aname, Warn = True); + return; +end if; + +Rewrite (N, + Make_Attribute_Definition_Clause (Loc, +Name = Get_Pragma_Arg (Arg2), +Chars = Aname, +Expression = Get_Pragma_Arg (Arg3))); +Analyze (N); + end Attribute_Definition; + --- -- AST_Entry -- --- @@ -15289,6 +15330,7 @@ Pragma_Assert_And_Cut = -1, Pragma_Assertion_Policy = 0, Pragma_Assume_No_Invalid_Values = 0, + Pragma_Attribute_Definition = +3, Pragma_Asynchronous = -1, Pragma_Atomic = 0, Pragma_Atomic_Components = 0, Index: sem_util.adb === --- sem_util.adb(revision 192918) +++ sem_util.adb(working copy) @@ -36,6 +36,7 @@ with Freeze; use Freeze; with Lib; use Lib; with Lib.Xref; use Lib.Xref; +with Namet.Sp; use Namet.Sp; with Nlists; use Nlists; with Nmake;use Nmake; with Output; use Output; @@ -404,6 +405,33 @@ and then Scope_Depth (ST) = Scope_Depth (SCT); end
[Ada] Fix thinko in bignum allocation routine
This makes the new support for extended overflow checking work on big-endian platforms as well by fixing a thinko in the allocation routine of bignums. No functional changes. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Eric Botcazou ebotca...@adacore.com * s-bignum.adb (Allocate_Bignum): Use the exact layout of Bignum_Data for the overlay. Index: s-bignum.adb === --- s-bignum.adb(revision 192933) +++ s-bignum.adb(working copy) @@ -233,14 +233,27 @@ pragma Import (Ada, BD); -- Expose a writable view of discriminant BD.Len so that we can --- initialize it. +-- initialize it. We need to use the exact layout of the record +-- for the overlay to shield ourselves from endianness issues. -BL : Length; -for BL'Address use BD.Len'Address; -pragma Import (Ada, BL); +type Bignum_Data_Header is record + Len : Length; + Neg : Boolean; +end record; +for Bignum_Data_Header use record + Len at 0 range 0 .. 23; + Neg at 3 range 0 .. 7; +end record; + +BDH : Bignum_Data_Header; +for BDH'Address use BD'Address; +pragma Import (Ada, BDH); + +pragma Assert (BDH.Len'Size = BD.Len'Size); + begin -BL := Len; +BDH.Len := Len; return B; end; end if;
[Ada] Incorrect sign extensions on IOCTL system constants
This change fixes incorrect values in s-oscons.ads on x86_64-freebsd, which were caused by those values being processed as signed, instead of unsigned, integers. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Thomas Quinot qui...@adacore.com * s-oscons-tmplt.c: Fix signedness of ioctl request identifiers for x86_64-freebsd. Index: s-oscons-tmplt.c === --- s-oscons-tmplt.c(revision 192918) +++ s-oscons-tmplt.c(working copy) @@ -218,6 +218,14 @@ #define CST(name,comment) C(#name,String,name,comment) +/* ioctl(2) requests are int in UNIX, but unsigned long on FreeBSD */ + +#ifdef __FreeBSD__ +# define CNI CNU +#else +# define CNI CND +#endif + #define STR(x) STR1(x) #define STR1(x) #x @@ -373,12 +381,12 @@ #ifndef FIONBIO # define FIONBIO -1 #endif -CND(FIONBIO, Set/clear non-blocking io) +CNI(FIONBIO, Set/clear non-blocking io) #ifndef FIONREAD # define FIONREAD -1 #endif -CND(FIONREAD, How many bytes to read) +CNI(FIONREAD, How many bytes to read) /* @@ -1318,6 +1326,7 @@ #define SIZEOF_sigset (sizeof (sigset_t)) CND(SIZEOF_sigset, sigset); #endif + /* -- Fields of struct msghdr @@ -1399,7 +1408,7 @@ /* There's no clock_gettime or clock_id's on Darwin, generate a dummy value */ # define CLOCK_RT_Ada -1 -#elif defined(FreeBSD) || defined(_AIX) +#elif defined(__FreeBSD__) || defined(_AIX) /** On these platforms use system provided monotonic clock instead of ** the default CLOCK_REALTIME. We then need to set up cond var attributes ** appropriately (see thread.c).
[Ada] Ignore Optimize_Alignment (Space) for packed variable length record
Normally pragma Optimize_Alignment (Space) forces an alignment of 1 for any packed record, but we can't do that for a variable length record, since we can't implement that case, so ignore the pragma in this case with a warning. The following test program shows the patch in action: 1. pragma optimize_alignment (space); 2. procedure testalign is 3.subtype Continuous_Values is float; 4.type Continuous_Values_Map is 5. array ( integer range ) of Continuous_Values; 6. 7.type my_record (s : integer) is record | Optimize_Alignment has no effect for my_record pragma is ignored for variable length record 8. x : continuous_values_map (1 .. s); 9.end record; 10.pragma pack (my_record); 11. 12.type my_record2 (s : integer) is record 13. x : continuous_values_map (1 .. 1000); 14.end record; 15.pragma pack (my_record2); 16. 17. begin 18. null; 19. end; Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Robert Dewar de...@adacore.com * layout.adb (Set_Composite_Alignment): Ignore pragma Optimize_Alignment (Space) for packed variable length records. Index: layout.adb === --- layout.adb (revision 192918) +++ layout.adb (working copy) @@ -2882,7 +2882,12 @@ and then Is_Packed (E) and then not Is_Atomic (E) then - Align := 1; + if not Size_Known_At_Compile_Time (E) then +Error_Msg_N (Optimize_Alignment has no effect for , E); +Error_Msg_N (\pragma is ignored for variable length record?, E); + else +Align := 1; + end if; -- Not a record, or not packed
[Patch] Add myself to MAINTAINERS as Write After Approval
Hi, This patch adds me to the Write After Approval section of MAINTAINERS. Regards, James Greenhalgh -- 2012-10-26 James Greenhalgh james.greenha...@arm.com * MAINTAINERS (Write After Approval): Add myself. wap.patch Description: wap.patch
PATCH: Correct alloca length in dump_gimple_bb_header
Hi,
When indent == 0, we call alloca with -1 bytes. This patch changes
it to indent + 1. This is a trunk only regression. OK to install?
Thanks.
H.J.
---
2012-10-29 H.J. Lu hongjiu...@intel.com
* gimple-pretty-print.c (dump_gimple_bb_header): Correct alloca
length.
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index 4b3235e..62c315e 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -2093,7 +2093,7 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int
indent, int flags)
gimple stmt = first_stmt (bb);
if (!stmt || gimple_code (stmt) != GIMPLE_LABEL)
{
- char *s_indent = (char *) alloca ((size_t) indent - 2 + 1);
+ char *s_indent = (char *) alloca ((size_t) indent + 1);
memset (s_indent, ' ', (size_t) indent);
s_indent[indent] = '\0';
fprintf (outf, %sbb %d:\n, s_indent, bb-index);
Re: PATCH: Correct alloca length in dump_gimple_bb_header
On Mon, Oct 29, 2012 at 05:06:01AM -0700, H.J. Lu wrote:
When indent == 0, we call alloca with -1 bytes. This patch changes
it to indent + 1. This is a trunk only regression. OK to install?
2012-10-29 H.J. Lu hongjiu...@intel.com
* gimple-pretty-print.c (dump_gimple_bb_header): Correct alloca
length.
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index 4b3235e..62c315e 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -2093,7 +2093,7 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int
indent, int flags)
gimple stmt = first_stmt (bb);
if (!stmt || gimple_code (stmt) != GIMPLE_LABEL)
{
- char *s_indent = (char *) alloca ((size_t) indent - 2 + 1);
+ char *s_indent = (char *) alloca ((size_t) indent + 1);
memset (s_indent, ' ', (size_t) indent);
s_indent[indent] = '\0';
fprintf (outf, %sbb %d:\n, s_indent, bb-index);
Can't you instead of all this just do
fprintf (outf, %*sbb %d:\n, indent, , bb-index);
?
Jakub
Re: User directed Function Multiversioning via Function Overloading (issue5752064)
Index: gcc/cgraph.c
===
--- gcc/cgraph.c (revision 192623)
+++ gcc/cgraph.c (working copy)
@@ -132,6 +132,74 @@ static GTY(()) struct cgraph_edge *free_edges;
/* Did procss_same_body_aliases run? */
bool same_body_aliases_done;
+/* Map a cgraph_node to cgraph_function_version_info using this htab.
+ The cgraph_function_version_info has a THIS_NODE field that is the
+ corresponding cgraph_node.. */
+htab_t GTY((param_is (struct cgraph_function_version_info *)))
+ cgraph_fnver_htab = NULL;
I think you want declare the htab static and arrange it to be freed after
cgraph construction, so you don't need to take care of nodes being removed
via the hooks.
OK with this change.
I have few other comments:
+ /* IFUNC resolvers have to be externally visible. */
+ TREE_PUBLIC (decl) = 1;
+ DECL_UNINLINABLE (decl) = 1;
Why the resolvers can not be inlined?
+
+ DECL_EXTERNAL (decl) = 0;
+ DECL_EXTERNAL (dispatch_decl) = 0;
+
+ DECL_CONTEXT (decl) = NULL_TREE;
+ DECL_INITIAL (decl) = make_node (BLOCK);
+ DECL_STATIC_CONSTRUCTOR (decl) = 0;
+ TREE_READONLY (decl) = 0;
+ DECL_PURE_P (decl) = 0;
I think those can be copied from the functions you are resolving. (well as well
as many attributes and properties)
+
+ if (DECL_COMDAT_GROUP (default_decl))
+{
+ DECL_COMDAT (decl) = DECL_COMDAT (default_decl);
+ make_decl_one_only (decl, DECL_COMDAT_GROUP (default_decl));
+}
+ else if (TREE_PUBLIC (default_decl))
+{
+ /* In this case, each translation unit with a call to this
+ versioned function will put out a resolver. Ensure it
+ is comdat to keep just one copy. */
+ DECL_COMDAT (decl) = 1;
+ make_decl_one_only (decl, DECL_ASSEMBLER_NAME (decl));
+}
+ /* Build result decl and add to function_decl. */
+ t = build_decl (UNKNOWN_LOCATION, RESULT_DECL, NULL_TREE, ptr_type_node);
+ DECL_ARTIFICIAL (t) = 1;
+ DECL_IGNORED_P (t) = 1;
+ DECL_RESULT (decl) = t;
+
+ gimplify_function_tree (decl);
+ push_cfun (DECL_STRUCT_FUNCTION (decl));
+ gimple_register_cfg_hooks ();
+ init_empty_tree_cfg_for_function (DECL_STRUCT_FUNCTION (decl));
+ cfun-curr_properties |=
+(PROP_gimple_lcf | PROP_gimple_leh | PROP_cfg | PROP_ssa
+ | PROP_gimple_any);
+ cfun-curr_properties = 15;
+ new_bb = create_empty_bb (ENTRY_BLOCK_PTR);
+ make_edge (ENTRY_BLOCK_PTR, new_bb, EDGE_FALLTHRU);
+ make_edge (new_bb, EXIT_BLOCK_PTR, 0);
+ *empty_bb = new_bb;
You can simplify this by init_lowered_empty_function.
Honza
Re: PATCH: Correct alloca length in dump_gimple_bb_header
On Mon, Oct 29, 2012 at 5:18 AM, Jakub Jelinek ja...@redhat.com wrote:
On Mon, Oct 29, 2012 at 05:06:01AM -0700, H.J. Lu wrote:
When indent == 0, we call alloca with -1 bytes. This patch changes
it to indent + 1. This is a trunk only regression. OK to install?
2012-10-29 H.J. Lu hongjiu...@intel.com
* gimple-pretty-print.c (dump_gimple_bb_header): Correct alloca
length.
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index 4b3235e..62c315e 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -2093,7 +2093,7 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int
indent, int flags)
gimple stmt = first_stmt (bb);
if (!stmt || gimple_code (stmt) != GIMPLE_LABEL)
{
- char *s_indent = (char *) alloca ((size_t) indent - 2 + 1);
+ char *s_indent = (char *) alloca ((size_t) indent + 1);
memset (s_indent, ' ', (size_t) indent);
s_indent[indent] = '\0';
fprintf (outf, %sbb %d:\n, s_indent, bb-index);
Can't you instead of all this just do
fprintf (outf, %*sbb %d:\n, indent, , bb-index);
?
The whole function has another:
char *s_indent = (char *) alloca ((size_t) indent + 1);
memset (s_indent, ' ', (size_t) indent);
s_indent[indent] = '\0';
We should make the same switch for both.
--
H.J.
Re: [Patch] Remove _GLIBCXX_HAVE_BROKEN_VSWPRINTF from mingw32-w64/os_defines.h
Fix the mail subject line. On 10/29/2012 06:56, JonY wrote: Hi, Workaround now exists for trunk mingw-w64 headers. Kai approved over IRC, so anybody with commit rights please push. ChangeLog 2012-10-29 Jonathan Yong jo...@users.sourceforge.net * config/os/mingw32-w64/os_defines.h: Remove _GLIBCXX_HAVE_BROKEN_VSWPRINTF as no longer required. Index: libstdc++-v3/config/os/mingw32-w64/os_defines.h === --- libstdc++-v3/config/os/mingw32-w64/os_defines.h (revision 192802) +++ libstdc++-v3/config/os/mingw32-w64/os_defines.h (working copy) @@ -63,8 +63,9 @@ // See libstdc++/20806. #define _GLIBCXX_HAVE_DOS_BASED_FILESYSTEM 1 -// See libstdc++/37522. -#define _GLIBCXX_HAVE_BROKEN_VSWPRINTF 1 +// See libstdc++/37522. mingw-w64 stdio redirect for C++ +// #define _GLIBCXX_HAVE_BROKEN_VSWPRINTF 1 +// Workaround added for mingw-w64 trunk headers r5437 // See libstdc++/43738 // On native windows targets there is no ioctl function. And the existing signature.asc Description: OpenPGP digital signature
Re: [patch] Unify bitmap interface.
On Thu, Oct 25, 2012 at 6:39 PM, Lawrence Crowl cr...@googlers.com wrote: The sbitmap non-bool returning bitwise operations have been merged with the bool versions. Sometimes this merge involved modifying the non-bool version to compute the bool value, and sometimes modifying bool version to add additional work from the non-bool version. The redundant routines have been ifdef'd out. I will remove the ifdef'd out code later. No #if 0 code, please. Let's just remove them. The allocation functions have not been renamed, because we often do not have an argument on which to overload. Would it work if we made the first argument a reference to the bitmap being allocated? I suppose this shouldn't be a big deal. The cardinality functions have not been renamed, because they have different parameters, and are thus not interchangable. Why not change the parameters then? The iteration functions have not been renamed, because they are functionally different. Functionally different? How? It seems like the patch only goes skin deep. I would rather make a true unification. If the only thing that remains the different are the allocation functions, that's not a big deal. But the point was to make everything else the same. I also notice that the patch does not rename any of the SET_BIT, RESET_BIT functions in sbitmap.c. Tested on x86_64. Config testing in progress. You will also want to test on a couple other architecture on the farm. By config testing, do you mean contrib/config-list.mk? Diego.
PATCH: PR rtl-optimization/55093: [4.8 Regression] [x32] -maddress-mode=long failed
Hi,
This patch changes get_elimination to check register number instead of
RTX. Tested on Linux/x32 with -maddress-mode=long. OK to install?
Thanks.
H.J.
---
gcc/
2012-10-29 H.J. Lu hongjiu...@intel.com
PR rtl-optimization/55093
* lra-eliminations.c (get_elimination): Check register number
instead of RTX.
gcc/testsuite/
2012-10-29 H.J. Lu hongjiu...@intel.com
PR rtl-optimization/55093
* gcc.target/i386/pr55093.c: New file.
diff --git a/gcc/lra-eliminations.c b/gcc/lra-eliminations.c
index d80..cbfbe7a 100644
--- a/gcc/lra-eliminations.c
+++ b/gcc/lra-eliminations.c
@@ -272,7 +272,7 @@ get_elimination (rtx reg)
if ((hard_regno = REGNO (reg)) 0 || hard_regno = FIRST_PSEUDO_REGISTER)
return NULL;
if ((ep = elimination_map[hard_regno]) != NULL)
-return ep-from_rtx != reg ? NULL : ep;
+return ep-from != hard_regno ? NULL : ep;
if ((offset = self_elim_offsets[hard_regno]) == 0)
return NULL;
/* This is an iteration to restore offsets just after HARD_REGNO
diff --git a/gcc/testsuite/gcc.target/i386/pr55093.c
b/gcc/testsuite/gcc.target/i386/pr55093.c
new file mode 100644
index 000..76b4042
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr55093.c
@@ -0,0 +1,80 @@
+/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-options -O2 -mx32 -maddress-mode=long } */
+
+typedef union tree_node *tree;
+typedef const union tree_node *const_tree;
+typedef struct {
+ unsigned long long low;
+ long long high;
+} double_int;
+struct real_value {
+};
+struct real_format {
+ int has_signed_zero;
+};
+extern const struct real_format * real_format_for_mode[];
+extern int real_isnegzero (const struct real_value *);
+enum tree_code { REAL_CST, SSA_NAME };
+struct tree_base {
+ enum tree_code code : 16;
+ union {
+unsigned int version;
+ }
+ u;
+};
+extern void tree_check_failed (const_tree, const char *, int, const char *,
...) __attribute__ ((__noreturn__));
+union tree_node {
+ struct tree_base base;
+};
+inline tree tree_check (tree __t, const char *__f, int __l, const char *__g,
enum tree_code __c) {
+ if (((enum tree_code) (__t)-base.code) != __c)
+tree_check_failed (__t, __f, __l, __g, __c, 0);
+ return __t;
+}
+struct prop_value_d {
+ int lattice_val;
+ tree value;
+ double_int mask;
+};
+typedef struct prop_value_d prop_value_t;
+static prop_value_t *const_val;
+static void canonicalize_float_value (prop_value_t *);
+typedef void (*ssa_prop_visit_stmt_fn) (prop_value_t);
+typedef void (*ssa_prop_visit_phi_fn) (void);
+typedef void (*ssa_prop_fold_stmt_fn) (void *gsi);
+typedef void (*ssa_prop_get_value_fn) ( prop_value_t *val);
+void ssa_propagate (ssa_prop_visit_stmt_fn, ssa_prop_visit_phi_fn);
+int substitute_and_fold (ssa_prop_get_value_fn, ssa_prop_fold_stmt_fn);
+void ccp_fold_stmt (void *);
+static void get_constant_value (prop_value_t *val) {
+ canonicalize_float_value (val);
+}
+static void canonicalize_float_value (prop_value_t *val) {
+ int mode;
+ struct real_value d;
+ if (val-lattice_val != 1
+ || ((enum tree_code) (val-value)-base.code) != REAL_CST)
+return;
+ mode = val-lattice_val;
+ if (real_format_for_mode[mode]-has_signed_zero real_isnegzero (d))
+ccp_fold_stmt (0);
+}
+static void set_lattice_value (tree var, prop_value_t new_val) {
+ prop_value_t *old_val = const_val[(tree_check ((var), ,
+ 0, ,
+ (SSA_NAME)))-base.u.version];
+ canonicalize_float_value (new_val);
+ canonicalize_float_value (old_val);
+}
+static void ccp_visit_phi_node (void) {
+ prop_value_t new_val;
+ set_lattice_value (0, new_val);
+}
+static void ccp_visit_stmt (prop_value_t v) {
+ set_lattice_value (0, v);
+}
+unsigned int do_ssa_ccp (void) {
+ ssa_propagate (ccp_visit_stmt, ccp_visit_phi_node);
+ substitute_and_fold (get_constant_value, ccp_fold_stmt);
+ return 0;
+}
Re: [PATCH] Fix debug info for expr and jump stmt
Hi, On Fri, 26 Oct 2012, Dehao Chen wrote: 1. abandon the changes in cfgexpand.c Well, you merely moved the bogus code to gimple-low.c. It is bogus because you unconditionally overwrite TREE_BLOCK of all operands (and all operands operands) with the statements block. I assume the unit-test you added shows the problem you were trying to fix? From the scan-assembler-no directive I'm not really sure what exactly the problem is you're seeing. Can you try to describe it with words for the example code? Which operands has no tree-block where it should have one, or which one has the wrong tree-block? Ciao, Michael.
Re: Make inliner to predict this-field to be optimized out
On Mon, Oct 29, 2012 at 1:03 AM, Jan Hubicka hubi...@ucw.cz wrote:
Hi,
this patch makes optimizer to predict this-field to be optimized out.
The main motivation for this is to make destructors that only calls destructor
of inner type to be inlined, even in the cold regions.
Bootstrapped/regtested x86_64-linux. Comitted.
2012-10-28 Jan Hubicka j...@suse.cz
* ipa-inline-analysis.c (eliminated_by_inlining_prob): Handle
this-field expressions.
Index: ipa-inline-analysis.c
===
--- ipa-inline-analysis.c (revision 192898)
+++ ipa-inline-analysis.c (working copy)
@@ -1567,6 +1567,7 @@ eliminated_by_inlining_prob (gimple stmt
if (gimple_assign_rhs_code (stmt) == CONVERT_EXPR
|| gimple_assign_rhs_code (stmt) == NOP_EXPR
|| gimple_assign_rhs_code (stmt) == VIEW_CONVERT_EXPR
+ || gimple_assign_rhs_code (stmt) == ADDR_EXPR
|| gimple_assign_rhs_class (stmt) == GIMPLE_SINGLE_RHS)
Please cache gimple_assign_rhs_code somewhere ...
{
tree rhs = gimple_assign_rhs1 (stmt);
@@ -1584,6 +1585,20 @@ eliminated_by_inlining_prob (gimple stmt
/* Reads of parameter are expected to be free. */
if (unmodified_parm (stmt, inner_rhs))
rhs_free = true;
+ /* Match expressions of form this-field. Those will most likely
+ combine with something upstream after inlining. */
+ else if (TREE_CODE (inner_rhs) == ADDR_EXPR)
+ {
+ tree op = get_base_address (TREE_OPERAND (inner_rhs, 0));
+ if (TREE_CODE (op) == PARM_DECL)
+ rhs_free = true;
+ else if (TREE_CODE (op) == MEM_REF)
+ {
+ op = get_base_address (TREE_OPERAND (op, 0));
Not necessary, just check
TREE_CODE (op) == SSA_NAME
unmodif ...
+ if (unmodified_parm (stmt, op))
+ rhs_free = true;
+ }
+ }
/* When parameter is not SSA register because its address is taken
and it is just copied into one, the statement will be
completely
Re: [patch] Unify bitmap interface.
On Mon, Oct 29, 2012 at 2:06 PM, Diego Novillo dnovi...@google.com wrote: On Thu, Oct 25, 2012 at 6:39 PM, Lawrence Crowl cr...@googlers.com wrote: The sbitmap non-bool returning bitwise operations have been merged with the bool versions. Sometimes this merge involved modifying the non-bool version to compute the bool value, and sometimes modifying bool version to add additional work from the non-bool version. The redundant routines have been ifdef'd out. I will remove the ifdef'd out code later. No #if 0 code, please. Let's just remove them. The allocation functions have not been renamed, because we often do not have an argument on which to overload. Would it work if we made the first argument a reference to the bitmap being allocated? I suppose this shouldn't be a big deal. It's definitely good to at least in one place see what kind of bitmap is actually being used ... ;) The cardinality functions have not been renamed, because they have different parameters, and are thus not interchangable. Why not change the parameters then? Agree, as a followup maybe. The iteration functions have not been renamed, because they are functionally different. Functionally different? How? It seems like the patch only goes skin deep. I would rather make a true unification. If the only thing that remains the different are the allocation functions, that's not a big deal. But the point was to make everything else the same. Indeed. I also notice that the patch does not rename any of the SET_BIT, RESET_BIT functions in sbitmap.c. They should go - a followup is fine though. Tested on x86_64. Config testing in progress. You will also want to test on a couple other architecture on the farm. By config testing, do you mean contrib/config-list.mk? Richard. Diego.
Re: real_zerop for vectors
On Sun, Oct 28, 2012 at 4:14 PM, Marc Glisse marc.gli...@inria.fr wrote:
Hello,
this patch lets some predicates on floating point constants answer true for
vectors, so optimizations are applied.
Tested bootstrap + testsuite (default languages).
Ok. Bonus points if you quickly eyed users of whether they may be surprised
in vectors coming through now.
Thanks,
Richard.
2012-10-29 Marc Glisse marc.gli...@inria.fr
PR middle-end/55027
gcc/
* tree.c (real_zerop, real_onep, real_twop, real_minus_onep):
Handle VECTOR_CST.
testsuite/
* gcc.dg/pr55027.c: New testcase.
--
Marc Glisse
Index: gcc/testsuite/gcc.dg/pr55027.c
===
--- gcc/testsuite/gcc.dg/pr55027.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr55027.c (revision 0)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options -Ofast -fdump-tree-optimized-raw } */
+
+typedef double v2df __attribute__ ((__vector_size__ (2 * sizeof
(double;
+
+void f (v2df *x)
+{
+ *x = 0 + 1 * *x;
+}
+
+/* { dg-final { scan-tree-dump-not gimple_assign optimized } } */
+/* { dg-final { cleanup-tree-dump optimized } } */
Property changes on: gcc/testsuite/gcc.dg/pr55027.c
___
Added: svn:keywords
+ Author Date Id Revision URL
Added: svn:eol-style
+ native
Index: gcc/tree.c
===
--- gcc/tree.c (revision 192894)
+++ gcc/tree.c (working copy)
@@ -1985,75 +1985,127 @@ tree_floor_log2 (const_tree expr)
}
/* Return 1 if EXPR is the real constant zero. Trailing zeroes matter for
decimal float constants, so don't return 1 for them. */
int
real_zerop (const_tree expr)
{
STRIP_NOPS (expr);
- return ((TREE_CODE (expr) == REAL_CST
- REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst0)
- !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr)
- || (TREE_CODE (expr) == COMPLEX_CST
- real_zerop (TREE_REALPART (expr))
- real_zerop (TREE_IMAGPART (expr;
+ switch (TREE_CODE (expr))
+{
+case REAL_CST:
+ return REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst0)
+ !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr;
+case COMPLEX_CST:
+ return real_zerop (TREE_REALPART (expr))
+ real_zerop (TREE_IMAGPART (expr));
+case VECTOR_CST:
+ {
+ unsigned i;
+ for (i = 0; i VECTOR_CST_NELTS (expr); ++i)
+ if (!real_zerop (VECTOR_CST_ELT (expr, i)))
+ return false;
+ return true;
+ }
+default:
+ return false;
+}
}
/* Return 1 if EXPR is the real constant one in real or complex form.
Trailing zeroes matter for decimal float constants, so don't return
1 for them. */
int
real_onep (const_tree expr)
{
STRIP_NOPS (expr);
- return ((TREE_CODE (expr) == REAL_CST
- REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst1)
- !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr)
- || (TREE_CODE (expr) == COMPLEX_CST
- real_onep (TREE_REALPART (expr))
- real_zerop (TREE_IMAGPART (expr;
+ switch (TREE_CODE (expr))
+{
+case REAL_CST:
+ return REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst1)
+ !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr;
+case COMPLEX_CST:
+ return real_onep (TREE_REALPART (expr))
+ real_zerop (TREE_IMAGPART (expr));
+case VECTOR_CST:
+ {
+ unsigned i;
+ for (i = 0; i VECTOR_CST_NELTS (expr); ++i)
+ if (!real_onep (VECTOR_CST_ELT (expr, i)))
+ return false;
+ return true;
+ }
+default:
+ return false;
+}
}
/* Return 1 if EXPR is the real constant two. Trailing zeroes matter
for decimal float constants, so don't return 1 for them. */
int
real_twop (const_tree expr)
{
STRIP_NOPS (expr);
- return ((TREE_CODE (expr) == REAL_CST
- REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst2)
- !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr)
- || (TREE_CODE (expr) == COMPLEX_CST
- real_twop (TREE_REALPART (expr))
- real_zerop (TREE_IMAGPART (expr;
+ switch (TREE_CODE (expr))
+{
+case REAL_CST:
+ return REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst2)
+ !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr;
+case COMPLEX_CST:
+ return real_twop (TREE_REALPART (expr))
+ real_zerop (TREE_IMAGPART (expr));
+case VECTOR_CST:
+ {
+ unsigned i;
+ for (i = 0; i VECTOR_CST_NELTS (expr); ++i)
+ if (!real_twop (VECTOR_CST_ELT (expr, i)))
+ return false;
+ return true;
+ }
+default:
+ return
Re: [PATCH] Fix CDDCE miscompilation (PR tree-optimization/55018)
On Sun, Oct 28, 2012 at 7:33 PM, Steven Bosscher stevenb@gmail.com wrote:
On Mon, Oct 22, 2012 at 11:09 PM, Jakub Jelinek wrote:
On Mon, Oct 22, 2012 at 10:51:43PM +0200, Steven Bosscher wrote:
Wouldn't it be way cheaper to just export dfs_find_deadend from cfganal.c
and call it in calc_dfs_tree on each unconnected bb?
I.e. (untested with the exception of the testcase):
2012-10-22 Jakub Jelinek ja...@redhat.com
PR tree-optimization/55018
* cfganal.c (dfs_find_deadend): No longer static.
* basic-block.h (dfs_find_deadend): New prototype.
* dominance.c (calc_dfs_tree): If saw_unconnected,
traverse from dfs_find_deadend of unconnected b
instead of b directly.
* gcc.dg/torture/pr55018.c: New test.
I have no better solution than this for the moment. I thought there
was a common DFS machinery in cfganal.c but there are actually many of
them, but unfortunately all doing things slightly different. Something
for the cleanup list for GCC 4.9...
We should use dfs_find_deadend in flow_dfs_compute_reverse_execute
also. This results in fewer fake edges created in
connect_infinite_loops_to_exit, especially for loops with multiple
dead ends.
(BTW, connect_infinite_loops_to_exit also connects other
reverse-unreachable points in the CFG to EXIT, so that calling
add_noreturn_fake_exit_edges and connect_infinite_loops_to_exit is
doing a bit of duplicate work -- another thing for the cleanup
list...)
Attached patch was bootstrappedtested on
{powerpc64,x86_64}-unknown-linux-gnu. OK?
Ok.
Thanks,
Richard.
Ciao!
Steven
Re: [PATCH] Fix debug info for expr and jump stmt
On Mon, Oct 29, 2012 at 2:51 PM, Michael Matz m...@suse.de wrote: Hi, On Fri, 26 Oct 2012, Dehao Chen wrote: 1. abandon the changes in cfgexpand.c Well, you merely moved the bogus code to gimple-low.c. It is bogus because you unconditionally overwrite TREE_BLOCK of all operands (and all operands operands) with the statements block. I assume the unit-test you added shows the problem you were trying to fix? From the scan-assembler-no directive I'm not really sure what exactly the problem is you're seeing. Can you try to describe it with words for the example code? Which operands has no tree-block where it should have one, or which one has the wrong tree-block? trees without block could be an issue when we extract them to some other statement (then without block), and move that to some random place. But the only issue should be that the stmt/expressions effective block becomes a different one (the currently active one during expansion). I don't see how we could end up generating too many block location DIEs because of this. Does the issue vanish when using -fno-tree-ter? And yes, I agree, the patch looks bogus. Richard. Ciao, Michael.
Re: [PATCH] Fix debug info for expr and jump stmt
Hi, On Mon, 29 Oct 2012, Richard Biener wrote: Well, you merely moved the bogus code to gimple-low.c. It is bogus because you unconditionally overwrite TREE_BLOCK of all operands (and all operands operands) with the statements block. I assume the unit-test you added shows the problem you were trying to fix? From the scan-assembler-no directive I'm not really sure what exactly the problem is you're seeing. Can you try to describe it with words for the example code? Which operands has no tree-block where it should have one, or which one has the wrong tree-block? trees without block could be an issue when we extract them to some other statement (then without block), and move that to some random place. Even then it would be either the frontends job to set the block, or the the job of the pass that introduced the new expression, when there is a clearly sane candidate for the block. But the only issue should be that the stmt/expressions effective block becomes a different one (the currently active one during expansion). Yep. I don't see how we could end up generating too many block location DIEs because of this. And even if, I don't see what's the _problem_ with too many block locations, except bloat. Ciao, Michael.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
Arnaud, After updating my tree to rev 192942 this morning, I'm getting a bootstrap failure with ada enabled: raised CONSTRAINT_ERROR : a-comlin.adb:65 explicit raise make[6]: *** [rts/s-oscons.ads] Error 1 make[6]: Leaving directory `bld-gcc/gcc/ada' make[5]: *** [gnatlib-shared-default] Error 2 I've configured with $ configure --enable-languages=all,ada,go,obj-c++ --enable-checking=release Does this look familiar? Thanks. Diego.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
After updating my tree to rev 192942 this morning, I'm getting a bootstrap failure with ada enabled: Looks like make did not rebuild (or too late?) xoscons. I'd suggest doing a rm -rf gcc/ada/bldtools from your obj directory and see if this fixes the error. If not, you might need to do a make -s -k clean Arno
Re: real_zerop for vectors
On Mon, 29 Oct 2012, Richard Biener wrote: On Sun, Oct 28, 2012 at 4:14 PM, Marc Glisse marc.gli...@inria.fr wrote: Hello, this patch lets some predicates on floating point constants answer true for vectors, so optimizations are applied. Tested bootstrap + testsuite (default languages). Ok. Bonus points if you quickly eyed users of whether they may be surprised in vectors coming through now. I did (that's why I didn't also change integer_twop), but I won't claim the bonus points, because in fold-const.c: * there is probably an issue with signed zero in fold_real_zero_addition_p * the RDIV_EXPR case would like real_zerop to mean at least one is zero * abs(x)=0 or 0 doesn't seem ready but that's not much compared to the broken integer vector optimizations ;-) -- Marc Glisse
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
On Mon, Oct 29, 2012 at 10:36 AM, Arnaud Charlet char...@adacore.com wrote: After updating my tree to rev 192942 this morning, I'm getting a bootstrap failure with ada enabled: Looks like make did not rebuild (or too late?) xoscons. I'd suggest doing a rm -rf gcc/ada/bldtools from your obj directory and see if this fixes the error. If not, you might need to do a make -s -k clean Well, this was a build from scratch. Maybe there is a parallel make bug in the new ada changes? Some undeclared dependency? Diego.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
Well, this was a build from scratch. Maybe there is a parallel make bug in the new ada changes? Some undeclared dependency? There is not in the recent changes. Maybe there's a latent bug, although I have never experienced it. At which revision are you? Arno
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
On Mon, Oct 29, 2012 at 10:39 AM, Arnaud Charlet char...@adacore.com wrote: Well, this was a build from scratch. Maybe there is a parallel make bug in the new ada changes? Some undeclared dependency? There is not in the recent changes. Maybe there's a latent bug, although I have never experienced it. At which revision are you? rev 192942. Removing bld/gcc/ada/bldtools did not help. I'm trying a -j1 build after doing 'make clean'. Diego.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
Well, this was a build from scratch. Maybe there is a parallel make bug in the new ada changes? Some undeclared dependency? There is not in the recent changes. Maybe there's a latent bug, although I have never experienced it. At which revision are you? rev 192942. Can you confirm that gcc/ada/Make-generated.in contains the following line: ./xoscons s-oscons ) ; \ and NOT: ./xoscons ) ; \ Arno
Re: patch to fix PR55106
On 12-10-28 8:49 PM, H.J. Lu wrote:
On Sun, Oct 28, 2012 at 5:43 PM, Vladimir Makarov vmaka...@redhat.com wrote:
The following patch fixes PR55106. A value in GENERAL_REGS is inherited
into a move with destination pseudo of SSE_REGS. It results into secondary
move for which inheritance is tried again an again. It means cycling LRA
passes.
The patch was successfully bootstrapped on x86/x86-64.
Committed as rev. 192904.
2012-10-28 Vladimir Makarov vmaka...@redhat.com
PR rtl-optimization/55106
* lra-constraints.c (skip_usage_debug_insns): New function.
(check_secondary_memory_needed_p): Ditto.
(inherit_reload_reg): Use the new functions. Improve debug
output.
Please add the testcase at
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55106#c1
I've committed the following patch.
2012-10-29 Vladimir Makarov vmaka...@redhat.com
PR rtl-optimization/55106
* g++.dg/pr55106.C: New.
Index: testsuite/g++.dg/pr55106.C
===
--- testsuite/g++.dg/pr55106.C (revision 0)
+++ testsuite/g++.dg/pr55106.C (working copy)
@@ -0,0 +1,67 @@
+/* { dg-do compile } */
+/* { dg-options -c -O3 } */
+templatetypename _Tp struct A {
+ typedef _Tp *pointer;
+ typedef _Tp reference;
+ typedef _Tp const_reference;
+ templatetypenamestruct rebind
+ {
+typedef A other;
+ };
+};
+
+templatetypename _Allocstruct __alloc_traits
+{
+ typedef typename _Alloc::pointer pointer;
+ typedef typename _Alloc::reference reference;
+ typedef typename _Alloc::const_reference const_reference;
+ templatetypename _Tpstruct rebind
+ {
+typedef typename _Alloc::template rebind_Tp::other other;
+ };
+};
+templatetypename _Tp, typename _Allocstruct B
+{
+ typedef typename __alloc_traits_Alloc::template rebind
+ _Tp::other _Tp_alloc_type;
+ typedef typename __alloc_traits_Tp_alloc_type::pointer pointer;
+ struct F
+ {
+pointer _M_start;
+ };
+ F _M_impl;
+};
+templatetypename _Tp, typename _Alloc = A_Tp class vec : B_Tp, _Alloc{
+ typedef B_Tp, _Alloc _Base;
+ typedef typename _Base::_Tp_alloc_type _Tp_alloc_type;
+ typedef __alloc_traits_Tp_alloc_type _Alloc_traits;
+
+public:
+ typedef _Tp value_type;
+ typedef typename _Alloc_traits::reference reference;
+ typedef typename _Alloc_traits::const_reference const_reference;
+ reference operator[](int p1)
+ {
+return *(this-_M_impl._M_start + p1);
+ }
+
+ const_reference operator[](long) const;
+};
+
+int a[17];
+class C {
+ vecint m_value;
+ void opModDivGuts(const C);
+ int mostSetBitP1() const;
+};
+void C::opModDivGuts(const C p1)
+{
+ int b = p1.mostSetBitP1(), c = b + 1;
+ int d[16];
+
+ for (int i = c; i; i--)
+a[i] = p1.m_value[i] b;
+
+ for (int i = 0; i c; i++)
+m_value[i] = d[i] b -b;
+}
Re: PATCH: PR rtl-optimization/55093: [4.8 Regression] [x32] -maddress-mode=long failed
On 12-10-29 9:06 AM, H.J. Lu wrote: Hi, This patch changes get_elimination to check register number instead of RTX. Tested on Linux/x32 with -maddress-mode=long. OK to install? Yes. Thanks, H.J.
Re: Trailing white spaces in LRA codes
On 12-10-28 11:03 PM, H.J. Lu wrote: Hi Vladimir, There are many trialling white spaces in LRA codes. This patch removes them. Thank you for pointing this out. I should be more careful with emacs macros using TAB. It would be nice if you install the patch.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
On Mon, Oct 29, 2012 at 10:44 AM, Arnaud Charlet char...@adacore.com wrote: Well, this was a build from scratch. Maybe there is a parallel make bug in the new ada changes? Some undeclared dependency? There is not in the recent changes. Maybe there's a latent bug, although I have never experienced it. At which revision are you? rev 192942. Can you confirm that gcc/ada/Make-generated.in contains the following line: ./xoscons s-oscons ) ; \ and NOT: ./xoscons ) ; \ Yup, it does: 94 $(OSCONS_CPP) ; \ 95 $(OSCONS_EXTRACT) ; \ 96 ./xoscons s-oscons ) ; \ 97 $(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/oscons/s-oscons.ads $(ADA_GEN_SUBDIR)/s-oscons.ads ; \ 98 $(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/oscons/s-oscons.h $(ADA_GEN_SUBDIR)/s-oscons.h
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
On Mon, Oct 29, 2012 at 10:48 AM, Diego Novillo dnovi...@google.com wrote: On Mon, Oct 29, 2012 at 10:44 AM, Arnaud Charlet char...@adacore.com wrote: Well, this was a build from scratch. Maybe there is a parallel make bug in the new ada changes? Some undeclared dependency? There is not in the recent changes. Maybe there's a latent bug, although I have never experienced it. At which revision are you? rev 192942. Can you confirm that gcc/ada/Make-generated.in contains the following line: ./xoscons s-oscons ) ; \ and NOT: ./xoscons ) ; \ Yup, it does: 94 $(OSCONS_CPP) ; \ 95 $(OSCONS_EXTRACT) ; \ 96 ./xoscons s-oscons ) ; \ 97 $(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/oscons/s-oscons.ads $(ADA_GEN_SUBDIR)/s-oscons.ads ; \ 98 $(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/oscons/s-oscons.h $(ADA_GEN_SUBDIR)/s-oscons.h
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
Yup, it does: OK then can you check the timestamp of ./gcc/ada/bldtools/oscons/xoscons vs e.g. ./gcc/ada/bldtools/oscons/xoscons/xoscons.adb and $(srcdir)/gcc/ada/xoscons.adb? Arno
Re: [PATCH 3/3] [asan] Instrument built-in memory access function calls
On Wed, 24 Oct 2012, Dodji Seketeli wrote: Jakub Jelinek ja...@redhat.com writes: On Tue, Oct 23, 2012 at 03:11:29PM +0200, Dodji Seketeli wrote: * asan.c (insert_if_then_before_iter) (instrument_mem_region_access) (maybe_instrument_builtin_call, maybe_instrument_call): New static Why not just write it: * asan.c (insert_if_then_before_iter, instrument_mem_region_access, maybe_instrument_builtin_call, maybe_instrument_call): New static ? It's emacs that formats it like that automatically. I am not sure how to teach him otherwise. I have fixed this as you want by doing it by hand. JFTR, the emacs-formatted-version *is* the correct one, at least when going by the GNU coding standard. Maybe something about being eager to balance parentheses as early as possible. :) But, I'll keep formatting it like that myself until someone updates the GCC coding standard. Hint, like. brgds, H-P
Re: Make inliner to predict this-field to be optimized out
{
tree rhs = gimple_assign_rhs1 (stmt);
@@ -1584,6 +1585,20 @@ eliminated_by_inlining_prob (gimple stmt
/* Reads of parameter are expected to be free. */
if (unmodified_parm (stmt, inner_rhs))
rhs_free = true;
+ /* Match expressions of form this-field. Those will most
likely
+ combine with something upstream after inlining. */
+ else if (TREE_CODE (inner_rhs) == ADDR_EXPR)
+ {
+ tree op = get_base_address (TREE_OPERAND (inner_rhs, 0));
+ if (TREE_CODE (op) == PARM_DECL)
+ rhs_free = true;
+ else if (TREE_CODE (op) == MEM_REF)
+ {
+ op = get_base_address (TREE_OPERAND (op, 0));
Not necessary, just check
TREE_CODE (op) == SSA_NAME
unmodif ...
Hmm, right, I can't have non-SSA there. Will fix that,
thanks!
Honza
Re: PATCH: PR rtl-optimization/55093: [4.8 Regression] [x32] -maddress-mode=long failed
H.J. Lu hjl.to...@gmail.com writes: Hi, This patch changes get_elimination to check register number instead of RTX. Tested on Linux/x32 with -maddress-mode=long. OK to install? FWIW, this doesn't sound right to me, at least not without more justification. The idea is that things like frame_pointer_rtx are supposed to be unique, so the original code: if ((ep = elimination_map[hard_regno]) != NULL) -return ep-from_rtx != reg ? NULL : ep; from != hard_regno ? NULL : ep; ought to be correct in itself. reload did the same thing: for (ep = reg_eliminate; ep reg_eliminate[NUM_ELIMINABLE_REGS]; ep++) if (ep-from_rtx == x ep-can_eliminate) return plus_constant (Pmode, ep-to_rtx, ep-previous_offset); It sounds on the face of it like the bug is elsewhere. Richard
Re: [RFC PATCH, i386]: Remove peephole2s for (subreg (operator (...)(...))) RTXes
Uros Bizjak ubiz...@gmail.com writes:
On Sun, Oct 28, 2012 at 2:37 AM, H.J. Lu hjl.to...@gmail.com wrote:
As suggested by Richard S. [1], after the patch that converts subreg:M
(op:N (...)(...)) to op:M (subreg:M (...) subreg:M (...)), we can
remove several peephole2 patterns that handle subregs of PLUS, MINUS
and MULT operators. I have attached RFC prototype patch that will
trigger an ICE when to-be-removed pattern triggers, with the intention
that these patterns wil be removed entirely (An invalid pattern was
indeed generated elsewhere, see patch).
I have committed following version that avoids all failures, reported by H.J.:
2012-10-29 Uros Bizjak ubiz...@gmail.com
* config/i386/i386.c (ix86_decompose_address): Use simplify_gen_subreg
to generate SImode equivalent of address, zero-extended with AND RTX.
* config/i386/i386.md (ashift to lea splitter): Split to SImode mult.
(simple lea to add/shift peephole2s): Remove peephole2s that operate
on subregs of DImode operations.
Re-tested on x86_64-pc-linux-gnu {,-m32} and committed to mainline SVN.
Thanks!
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
On Mon, Oct 29, 2012 at 10:51 AM, Arnaud Charlet char...@adacore.com wrote: Yup, it does: OK then can you check the timestamp of ./gcc/ada/bldtools/oscons/xoscons vs e.g. ./gcc/ada/bldtools/oscons/xoscons/xoscons.adb and $(srcdir)/gcc/ada/xoscons.adb? I have no directory called gcc/ada/bldtools/oscons/xoscons. That's an executable. I suppose you meant gcc/ada/bldtools/oscons/xoscons.adb? $ ls -l gcc/ada/bldtools/oscons/xoscons gcc/ada/bldtools/oscons/xoscons.adb ~/gcc/clean/trunk/gcc/ada/xoscons.adb -rw-r- 1 dnovillo eng 17767 Oct 29 08:32 /home/dnovillo/gcc/clean/trunk/gcc/ada/xoscons.adb -rwxr-x--- 1 dnovillo eng 59793 Oct 29 10:38 gcc/ada/bldtools/oscons/xoscons* -rw-r- 1 dnovillo eng 17767 Oct 29 08:32 gcc/ada/bldtools/oscons/xoscons.adb Still happens with -j1 (after starting from scratch), so it's probably not a parallel make issue. We chatted about this on IRC, and Richi is seeing the same problem on his builds. Thanks. Diego.
Re: [PATCH] Fix debug info for expr and jump stmt
On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz m...@suse.de wrote:
Hi,
On Mon, 29 Oct 2012, Richard Biener wrote:
Well, you merely moved the bogus code to gimple-low.c. It is bogus
because you unconditionally overwrite TREE_BLOCK of all operands (and all
Emm, then in gimple-low.c, we should probably not unconditionally
overwrite gimple_block for stmts too?
operands operands) with the statements block. I assume the unit-test you
added shows the problem you were trying to fix?
From the scan-assembler-no directive I'm not really sure what exactly the
problem is you're seeing. Can you try to describe it with words for the
example code? Which operands has no tree-block where it should have one,
or which one has the wrong tree-block?
trees without block could be an issue when we extract them to some other
statement (then without block), and move that to some random place.
Even then it would be either the frontends job to set the block, or the
the job of the pass that introduced the new expression, when there is a
clearly sane candidate for the block.
Please correct me if I'm wrong: front-end does not set blocks for
either stmt/expr. lower_stmt is the first place that block is set for
stmt, thus I think it should also be the first place to set block for
expr.
But the only issue should be that the stmt/expressions effective block
becomes a different one (the currently active one during expansion).
I agree. Initially, both stmt and expressions is set to NULL block.
Whenever we update the stmt's block, we should also update the
expression's block, otherwise they will be inconsistent.
Yep.
I don't see how we could end up generating too many block location
DIEs because of this.
For the unittest I added, all the assembly code guarded by if are
should be within one lexical block, which is lock:
LBB1
{
asm1
asm2
asm3
asm4
}
However, because some expression are with NULL lexical block, these
assembly codes are separated by several discontinual lexical block
which is like:
LBB1
{
asm1
}
asm2
LBB2
{
asm3
}
asm4
And even if, I don't see what's the _problem_ with too many block
locations, except bloat.
Yes, bloat is the major problem.
Thanks,
Dehao
Ciao,
Michael.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
I have no directory called gcc/ada/bldtools/oscons/xoscons. That's an executable. Yes, that's as expected, I asked you to check the timestamp of this file. Still happens with -j1 (after starting from scratch), so it's probably not a parallel make issue. Right, no surprise here. We chatted about this on IRC, and Richi is seeing the same problem on his builds. Very strange. I'm looking at it. Arno
Re: [PR54693] loss of debug info in jump threading and loop ivopts
On Oct 26, 2012, Jakub Jelinek ja...@redhat.com wrote: On Fri, Oct 26, 2012 at 04:30:41AM -0200, Alexandre Oliva wrote: From: Alexandre Oliva lxol...@fsfla.org Fixed .git/config and the patches to use my @redhat.com address for these patches. +++ b/gcc/testsuite/gcc.dg/guality/pr54693.c The #include stdlib.h looks useless to me, please remove it and adjust gdb-test line accordingly. Thanks for catching this, it was a left-over from a failed experiment. Don't we always ignore gimple_location on debug_stmts anyway? Right now, we mostly do, but they may become relevant once stmt frontier notes are implemented. + gimple def_temp = gimple_build_debug_bind (vexpr, comp, NULL); Just wonder whether the above is sufficient It surely works. It may prevent common subexpression equivalences from being detected by VTA, but it also saves some temps. The exprs are of linear scaling, so they should be relatively small, but I can't guess whether there'd be any measurable benefit from splitting them up. Thanks for the reviews. -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist Red Hat Brazil Compiler Engineer
Re: [PATCH] Fix libbacktrace on 32-bit sparc
On Sat, Oct 27, 2012 at 10:12 PM, David Miller da...@davemloft.net wrote: From: David Miller da...@davemloft.net Date: Sun, 28 Oct 2012 00:31:27 -0400 (EDT) The size is 24, and my patch definitely makes the crashes go away. It seems like a vector is being used for a mixed set of objects. I'll try to figure out how that is happening. Ok, the problem seems to have to do with releases. The releases place vector memory chunks into a global pool. So a memory chunk from a vector used for one type of object, can be sucked into and used by another vector. But the alignment requirements are different, so we can obtain a chunk from the freelist that was being used for a vector of 4-byte aligned objects. The crash sequences are always of the form: vec_release(0xffb37ac8) base+size(0xf0199008) amount(312) ... vec_grow(0xffb37ac8:24) from 0x975168, ret=0xf01754cc [size(24):alc(360)] That size alignment done by backtrace_alloc() has no influence upon this issue. Since chunks are released from wherever the vector's allocation point was at the time of the release. In fact I bet that alignment in backtrace_alloc() never triggers when it is invoked from backtrace_vector_grow(). Thanks for tracking it down. This patch should fix it. Bootstrapped and ran libbacktrace testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Ian 2012-10-29 Ian Lance Taylor i...@google.com * mmap.c (backtrace_vector_release): Make sure freed block is aligned on 8-byte boundary. foo.patch Description: Binary data
Minor update for inlining for code size
Hi,
this patch updates inlining functions called once into inlining functions
called multiple times when the overall unit size for inlining everywhere
shriks. This is now quite possible in the cases where inline predicates catch
some DCE as seen in the silly testcase attached. We already caught most of
instances of those as part of inlining small functions but in rarer cases where
the function is too large we mis the oppurtunity.
Bootstrapped/regtested x86_64-linux, will commit it shortly.
Honza
* gcc.dg/ipa/inline-6.c: New testcase.
* ipa-inline.c (want_inline_function_called_once_p): Rename to ...
(want_inline_function_to_all_callers_p): check also functions with
multiple callers.
(ipa_inline): Handle inlining for size into multiple callers.
Index: ipa-inline.c
===
*** ipa-inline.c(revision 192937)
--- ipa-inline.c(working copy)
*** check_caller_edge (struct cgraph_node *n
*** 681,714
}
! /* Decide if NODE is called once inlining it would eliminate need
!for the offline copy of function. */
static bool
! want_inline_function_called_once_p (struct cgraph_node *node)
{
struct cgraph_node *function = cgraph_function_or_thunk_node (node, NULL);
/* Already inlined? */
if (function-global.inlined_to)
return false;
!/* Zero or more then one callers? */
!if (!node-callers
!|| node-callers-next_caller)
return false;
/* Maybe other aliases has more direct calls. */
if (cgraph_for_node_and_aliases (node, check_caller_edge, node-callers,
true))
return false;
!/* Recursive call makes no sense to inline. */
!if (cgraph_edge_recursive_p (node-callers))
! return false;
!/* External functions are not really in the unit, so inlining
! them when called once would just increase the program size. */
!if (DECL_EXTERNAL (function-symbol.decl))
! return false;
!/* Offline body must be optimized out. */
!if (!cgraph_will_be_removed_from_program_if_no_direct_calls (function))
! return false;
!if (!can_inline_edge_p (node-callers, true))
return false;
return true;
}
--- 681,721
}
! /* Decide if inlining NODE would reduce unit size by eliminating
!the offline copy of function.
!When COLD is true the cold calls are considered, too. */
static bool
! want_inline_function_to_all_callers_p (struct cgraph_node *node, bool cold)
{
struct cgraph_node *function = cgraph_function_or_thunk_node (node, NULL);
+struct cgraph_edge *e;
+bool has_hot_call = false;
+
+/* Does it have callers? */
+if (!node-callers)
+ return false;
/* Already inlined? */
if (function-global.inlined_to)
return false;
!if (cgraph_function_or_thunk_node (node, NULL) != node)
! return false;
!/* Inlining into all callers would increase size? */
!if (estimate_growth (node) 0)
return false;
/* Maybe other aliases has more direct calls. */
if (cgraph_for_node_and_aliases (node, check_caller_edge, node-callers,
true))
return false;
!/* All inlines must be possible. */
!for (e = node-callers; e; e = e-next_caller)
! {
!if (!can_inline_edge_p (e, true))
! return false;
!if (!has_hot_call cgraph_maybe_hot_edge_p (e))
!has_hot_call = 1;
! }
!
!if (!cold !has_hot_call)
return false;
return true;
}
*** ipa_inline (void)
*** 1729,1742
symtab_remove_unreachable_nodes (true, dump_file);
free (order);
! /* We already perform some inlining of functions called once during
! inlining small functions above. After unreachable nodes are removed,
! we still might do a quick check that nothing new is found. */
if (flag_inline_functions_called_once)
{
int cold;
if (dump_file)
! fprintf (dump_file, \nDeciding on functions called once:\n);
/* Inlining one function called once has good chance of preventing
inlining other function into the same callee. Ideally we should
--- 1736,1751
symtab_remove_unreachable_nodes (true, dump_file);
free (order);
! /* Inline functions with a property that after inlining into all callers the
! code size will shrink because the out-of-line copy is eliminated.
! We do this regardless on the callee size as long as function growth
limits
! are met. */
if (flag_inline_functions_called_once)
{
int cold;
if (dump_file)
! fprintf (dump_file,
!\nDeciding on functions to be inlined into all callers:\n);
/* Inlining one function called once has good chance of preventing
inlining other function into the same callee. Ideally we should
*** ipa_inline (void)
*** 1757,1787
{
Re: [PATCH] gcc-{ar,nm,ranlib}: Find binutils binaries relative to self
Ping ^ 3.
On 10/18/2012 10:30 AM, Meador Inge wrote:
Ping ^ 2.
On 10/09/2012 09:44 PM, Meador Inge wrote:
Ping.
On 10/04/2012 03:45 PM, Meador Inge wrote:
Hi All,
Currently the gcc-{ar,nm,ranlib} utilities assume that binutils is in
path when invoking the wrapped binutils program. This goes against the
accepted practice in GCC to find sub-programs relative to where the
GCC binaries are stored and to not make assumptions about the PATH.
This patch changes the gcc-{ar,nm,ranlib} utilities to do the same
by factoring out some utility code for finding files from collect2.c.
These functions are then leveraged to find the binutils programs.
Note that similar code exist in gcc.c. Perhaps one day everything
can be merged to the file-find files.
Tested for Windows and GNU/Linux hosts and i686-pc-linux-gnu and
arm-none-eabi targets. OK?
P.S. I am not quite sure what is best for the copyrights and contributed
by comments in the file-find* files I added since that code was just moved.
This patch drops the contributed by and keeps all the copyright dates from
collect2.c.
2012-10-04 Meador Inge mead...@codesourcery.com
* collect2.c (main): Call find_file_set_debug.
(find_a_find, add_prefix, prefix_from_env, prefix_from_string):
Factor out into ...
* file-find.c (New file): ... here and ...
* file-find.h (New file): ... here.
* gcc-ar.c (standard_exec_prefix): New variable.
(standard_libexec_prefix): Ditto.
(tooldir_base_prefix) Ditto.
(self_exec_prefix): Ditto.
(self_libexec_prefix): Ditto.
(self_tooldir_prefix): Ditto.
(target_version): Ditto.
(path): Ditto.
(target_path): Ditto.
(setup_prefixes): New function.
(main): Rework how wrapped programs are found.
* Makefile.in (OBJS-libcommon-target): Add file-find.o.
(AR_OBJS): New variable.
(gcc-ar$(exeext)): Add dependency on $(AR_OBJS).
(gcc-nm$(exeext)): Ditto.
(gcc-ranlib(exeext)): Ditto.
(COLLECT2_OBJS): Add file-find.o.
(collect2.o): Add file-find.h prerequisite.
(file-find.o): New rule.
Index: gcc/gcc-ar.c
===
--- gcc/gcc-ar.c(revision 192099)
+++ gcc/gcc-ar.c(working copy)
@@ -21,21 +21,110 @@
#include config.h
#include system.h
#include libiberty.h
+#include file-find.h
#ifndef PERSONALITY
#error Please set personality
#endif
+/* The exec prefix as derived at compile-time from --prefix. */
+
+static const char standard_exec_prefix[] = STANDARD_EXEC_PREFIX;
+
+/* The libexec prefix as derived at compile-time from --prefix. */
+
static const char standard_libexec_prefix[] = STANDARD_LIBEXEC_PREFIX;
+
+/* The bindir prefix as derived at compile-time from --prefix. */
+
static const char standard_bin_prefix[] = STANDARD_BINDIR_PREFIX;
-static const char *const target_machine = TARGET_MACHINE;
+/* A relative path to be used in finding the location of tools
+ relative to this program. */
+
+static const char *const tooldir_base_prefix = TOOLDIR_BASE_PREFIX;
+
+/* The exec prefix as relocated from the location of this program. */
+
+static const char *self_exec_prefix;
+
+/* The libexec prefix as relocated from the location of this program. */
+
+static const char *self_libexec_prefix;
+
+/* The tools prefix as relocated from the location of this program. */
+
+static const char *self_tooldir_prefix;
+
+/* The name of the machine that is being targeted. */
+
+static const char *const target_machine = DEFAULT_TARGET_MACHINE;
+
+/* The target version. */
+
+static const char *const target_version = DEFAULT_TARGET_VERSION;
+
+/* The collection of target specific path prefixes. */
+
+static struct path_prefix target_path;
+
+/* The collection path prefixes. */
+
+static struct path_prefix path;
+
+/* The directory separator. */
+
static const char dir_separator[] = { DIR_SEPARATOR, 0 };
+static void
+setup_prefixes (const char *exec_path)
+{
+ const char *self;
+
+ self = getenv (GCC_EXEC_PREFIX);
+ if (!self)
+self = exec_path;
+ else
+self = concat (self, gcc- PERSONALITY, NULL);
+
+ /* Relocate the exec prefix. */
+ self_exec_prefix = make_relative_prefix (self,
+ standard_bin_prefix,
+ standard_exec_prefix);
+ if (self_exec_prefix == NULL)
+self_exec_prefix = standard_exec_prefix;
+
+ /* Relocate libexec prefix. */
+ self_libexec_prefix = make_relative_prefix (self,
+ standard_bin_prefix,
+ standard_libexec_prefix);
+ if (self_libexec_prefix == NULL)
+self_libexec_prefix = standard_libexec_prefix;
+
+
+ /* Build the relative path to the target-specific tool directory. */
+ self_tooldir_prefix = concat (tooldir_base_prefix, target_machine,
+
RFA: patch to fix PR55116
H.J. in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55116 reported an interesting address (and:DI (subreg:DI (plus:SI (ashift:SI (reg:SI 96 [ glob_vol_int.22 ]) (const_int 2 [0x2])) (symbol_ref:SI (glob_vol_int_arr) var_decl 0x703c2720 glob_vol_int_arr)) 0) (const_int 4294967295 [0x])) which can not be correctly extracted. Here `and' with `subreg' behaves as an address mutation. The following patch fixes the problem. Ok to commit, Richard? 2012-10-29 Vladimir Makarov vmaka...@redhat.com PR middle-end/55116 * rtlanal.c (strip_address_mutation): Add SUBREG case. Index: rtlanal.c === --- rtlanal.c (revision 192942) +++ rtlanal.c (working copy) @@ -5459,6 +5459,11 @@ strip_address_mutations (rtx *loc, enum else if (code == AND CONST_INT_P (XEXP (*loc, 1))) /* (and ... (const_int -X)) is used to align to X bytes. */ loc = XEXP (*loc, 0); + else if (code == SUBREG + ! REG_P (XEXP (*loc, 0)) ! MEM_P (XEXP (*loc, 0))) + /* (subreg (operator ...) ...) usually inside and is used for + mode conversion too. */ + loc = XEXP (*loc, 0); else return loc; if (outer_code)
Re: [PATCH] Fix debug info for expr and jump stmt
On Mon, Oct 29, 2012 at 4:25 PM, Dehao Chen de...@google.com wrote:
On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz m...@suse.de wrote:
Hi,
On Mon, 29 Oct 2012, Richard Biener wrote:
Well, you merely moved the bogus code to gimple-low.c. It is bogus
because you unconditionally overwrite TREE_BLOCK of all operands (and all
Emm, then in gimple-low.c, we should probably not unconditionally
overwrite gimple_block for stmts too?
gimple stmts have no block before gimple-low.
operands operands) with the statements block. I assume the unit-test you
added shows the problem you were trying to fix?
From the scan-assembler-no directive I'm not really sure what exactly the
problem is you're seeing. Can you try to describe it with words for the
example code? Which operands has no tree-block where it should have one,
or which one has the wrong tree-block?
trees without block could be an issue when we extract them to some other
statement (then without block), and move that to some random place.
Even then it would be either the frontends job to set the block, or the
the job of the pass that introduced the new expression, when there is a
clearly sane candidate for the block.
Please correct me if I'm wrong: front-end does not set blocks for
either stmt/expr. lower_stmt is the first place that block is set for
stmt, thus I think it should also be the first place to set block for
expr.
No, only the block on the gimple stmt matters for gimple. You should not
need to touch the operands block.
But the only issue should be that the stmt/expressions effective block
becomes a different one (the currently active one during expansion).
I agree. Initially, both stmt and expressions is set to NULL block.
Whenever we update the stmt's block, we should also update the
expression's block, otherwise they will be inconsistent.
I don't think so.
Yep.
I don't see how we could end up generating too many block location
DIEs because of this.
For the unittest I added, all the assembly code guarded by if are
should be within one lexical block, which is lock:
LBB1
{
asm1
asm2
asm3
asm4
}
However, because some expression are with NULL lexical block, these
assembly codes are separated by several discontinual lexical block
which is like:
LBB1
{
asm1
}
asm2
asm2 should have gone into the currently open scope (similiar for what
we do for line number information).
LBB2
{
asm3
}
asm4
And even if, I don't see what's the _problem_ with too many block
locations, except bloat.
Yes, bloat is the major problem.
Thanks,
Dehao
Ciao,
Michael.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
We chatted about this on IRC, and Richi is seeing the same problem on his builds. Very strange. I'm looking at it. OK, this comes from code duplicated between gcc-interface/Makefile.in and Make-generated.in, pretty messy. Testing the patch below, will commit if success. 2012-10-29 Arnaud Charlet char...@adacore.com * gcc-interface/Makefile.in (s-oscons.ads): Adjust call to xoscons. Arno -- --- gcc-interface/Makefile.in (revision 192944) +++ gcc-interface/Makefile.in (working copy) @@ -2610,7 +2610,7 @@ $(RTSDIR)/s-oscons.ads: ../stamp-gnatlib (cd $(RTSDIR) ; \ $(OSCONS_CPP) ; \ $(OSCONS_EXTRACT) ; \ - ../bldtools/oscons/xoscons) + ../bldtools/oscons/xoscons s-oscons) # Don't use semicolon separated shell commands that involve list expansions. # The semicolon triggers a call to DCL on VMS and DCL can't handle command
Re: RFA: patch to fix PR55116
Vladimir Makarov vmaka...@redhat.com writes: H.J. in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55116 reported an interesting address (and:DI (subreg:DI (plus:SI (ashift:SI (reg:SI 96 [ glob_vol_int.22 ]) (const_int 2 [0x2])) (symbol_ref:SI (glob_vol_int_arr) var_decl 0x703c2720 glob_vol_int_arr)) 0) (const_int 4294967295 [0x])) which can not be correctly extracted. Here `and' with `subreg' behaves as an address mutation. The following patch fixes the problem. Ok to commit, Richard? Heh, I wondered if subregs might still be used like that, and was almost tempted to add them just in case. I think this particular case is really a failed canonicalisation and that: (and:DI (subreg:DI (foo:SI ...) 0) (const_int 0x)) ought to be: (zero_extend:DI (foo:SI ...)) But I know I've approved MIPS patches to accept (and:DI ... (const_int 0x)) as an alternative. Index: rtlanal.c === --- rtlanal.c (revision 192942) +++ rtlanal.c (working copy) @@ -5459,6 +5459,11 @@ strip_address_mutations (rtx *loc, enum else if (code == AND CONST_INT_P (XEXP (*loc, 1))) /* (and ... (const_int -X)) is used to align to X bytes. */ loc = XEXP (*loc, 0); + else if (code == SUBREG + ! REG_P (XEXP (*loc, 0)) ! MEM_P (XEXP (*loc, 0))) + /* (subreg (operator ...) ...) usually inside and is used for + mode conversion too. */ + loc = XEXP (*loc, 0); I think the condition should be: else if (code == SUBREG !OBJECT_P (SUBREG_REG (*loc)) subreg_lowpart (*loc)) OK with that change, if it works. Thanks for fixing this. Richard
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
Testing the patch below, will commit if success. 2012-10-29 Arnaud Charlet char...@adacore.com * gcc-interface/Makefile.in (s-oscons.ads): Adjust call to xoscons. Committed on trunk now. Sorry for the breakage, my build was incremental and missed this missing part. Arno
Re: RFA: patch to fix PR55116
Sorry, forgot a line: Richard Sandiford rdsandif...@googlemail.com writes: Vladimir Makarov vmaka...@redhat.com writes: Index: rtlanal.c === --- rtlanal.c (revision 192942) +++ rtlanal.c (working copy) @@ -5459,6 +5459,11 @@ strip_address_mutations (rtx *loc, enum else if (code == AND CONST_INT_P (XEXP (*loc, 1))) /* (and ... (const_int -X)) is used to align to X bytes. */ loc = XEXP (*loc, 0); + else if (code == SUBREG + ! REG_P (XEXP (*loc, 0)) ! MEM_P (XEXP (*loc, 0))) + /* (subreg (operator ...) ...) usually inside and is used for + mode conversion too. */ + loc = XEXP (*loc, 0); I think the condition should be: else if (code == SUBREG !OBJECT_P (SUBREG_REG (*loc)) subreg_lowpart (*loc)) loc = SUBREG_REG (*loc); i.e. please use SUBREG_REG rather than XEXP.
Re: Fix estimation of array accesses
ICK ...
Why not sth as simple as
return num_ssa_operands (stmt, SSA_OP_USE);
? a[1][2] and b[2] really have the same cost, variable length
objects have extra SSA operands in ARRAY_REF/COMPONENT_REF for
the size. Thus, stmt cost somehow should reflect the number
of dependent stmts.
So in estimate_num_insns I'd try
int
estimate_num_insns (gimple stmt, eni_weights *weights)
{
unsigned cost, i;
enum gimple_code code = gimple_code (stmt);
tree lhs;
tree rhs;
switch (code)
{
case GIMPLE_ASSIGN:
/* Initialize with the number of SSA uses, one is free. */
cost = num_ssa_operands (stmt, SSA_OP_USE);
if (cost 1)
--cost;
Hi,
this is the udpated patch I am testing after today discussion. I decided to
drop the ipa-inline-analysis changes and do that incrementally. So the patch
now trashes tramp3d performance by increasing need for early-inlining-insns,
but it is not inexpected.
The patch also fixes accounting of addr expr that got previously confused with
a load.
Does this seem sane?
* tree-inline.c (estimate_operator_cost): Return 1 for non-trivial
ADDR_EXPR operations.
(estimate_num_insns): Do not confuse general single rhs with
loads; account cost of non-trivial addr_expr for ASSIGN_EXPR,
GIMPLE_RETURN and GIMPLE_ASM
Index: tree-inline.c
===
--- tree-inline.c (revision 192945)
+++ tree-inline.c (working copy)
@@ -3447,6 +3447,19 @@ estimate_operator_cost (enum tree_code c
if (TREE_CODE (op2) != INTEGER_CST)
return weights-div_mod_cost;
return 1;
+case ADDR_EXPR:
+ {
+tree addr_base;
+HOST_WIDE_INT addr_offset;
+
+ addr_base = get_addr_base_and_unit_offset (TREE_OPERAND (op1, 0),
+ addr_offset);
+ /* If the offset is variable or with non-zero offset, return 2. */
+ if (!addr_base || addr_offset != 0 || TREE_CODE (addr_base) != MEM_REF
+ || !integer_zerop (TREE_OPERAND (addr_base, 1)))
+ return 1;
+ }
+ return 0;
default:
/* We expect a copy assignment with no operator. */
@@ -3512,14 +3525,24 @@ estimate_num_insns (gimple stmt, eni_wei
lhs = gimple_assign_lhs (stmt);
rhs = gimple_assign_rhs1 (stmt);
- if (is_gimple_reg (lhs))
- cost = 0;
- else
+ /* Store. */
+ if (gimple_vdef (stmt))
cost = estimate_move_cost (TREE_TYPE (lhs));
+ else
+ cost = 0;
- if (!is_gimple_reg (rhs) !is_gimple_min_invariant (rhs))
+ /* Load. */
+ if (gimple_vuse (stmt))
cost += estimate_move_cost (TREE_TYPE (rhs));
+ /* Stores, loads and address expressions may have variable array
+references in them. Account these. */
+ if (gimple_vuse (stmt))
+ cost += num_ssa_operands (stmt, SSA_OP_USE);
+ else if (gimple_vdef (stmt)
+ || gimple_assign_rhs_code (stmt) == ADDR_EXPR)
+ cost += MAX (0, (int)num_ssa_operands (stmt, SSA_OP_USE) - 1);
+
cost += estimate_operator_cost (gimple_assign_rhs_code (stmt), weights,
gimple_assign_rhs1 (stmt),
get_gimple_rhs_class
(gimple_assign_rhs_code (stmt))
@@ -3596,6 +3619,7 @@ estimate_num_insns (gimple stmt, eni_wei
case GIMPLE_RETURN:
return weights-return_cost;
++ MAX (0, (int)num_ssa_operands (stmt, SSA_OP_USE) - 1);
case GIMPLE_GOTO:
case GIMPLE_LABEL:
@@ -3606,7 +3630,10 @@ estimate_num_insns (gimple stmt, eni_wei
return 0;
case GIMPLE_ASM:
- return asm_str_count (gimple_asm_string (stmt));
+ return (asm_str_count (gimple_asm_string (stmt))
+ /* Account also possible non-trivial addr_exprs
+in the arguments. */
+ + num_ssa_operands (stmt, SSA_OP_USE));
case GIMPLE_RESX:
/* This is either going to be an external function call with one
Re: [PATCH] Fix debug info for expr and jump stmt
Hi,
On Mon, 29 Oct 2012, Dehao Chen wrote:
On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz m...@suse.de wrote:
Hi,
On Mon, 29 Oct 2012, Richard Biener wrote:
Well, you merely moved the bogus code to gimple-low.c. It is bogus
because you unconditionally overwrite TREE_BLOCK of all operands (and all
Emm, then in gimple-low.c, we should probably not unconditionally
overwrite gimple_block for stmts too?
That's okay, because the gimple statements were just generated during
gimplification.
trees without block could be an issue when we extract them to some
other statement (then without block), and move that to some random
place.
Even then it would be either the frontends job to set the block, or
the the job of the pass that introduced the new expression, when there
is a clearly sane candidate for the block.
Please correct me if I'm wrong: front-end does not set blocks for
either stmt/expr.
Yes, sorry, you're right. It's the lowering of bind expressions that
tacks blocks to statements.
lower_stmt is the first place that block is set for stmt, thus I think
it should also be the first place to set block for expr.
The point is, there shouldn't be any expressions left after gimplification
that would receive locations/blocks. Every side-effect of computing an
expression should be a separate gimple statement. The only exception
should be loads for arguments of call and asm statements statements and
there setting the location of those expressions to the one of the call
statement itself is superfluous.
But the only issue should be that the stmt/expressions effective block
becomes a different one (the currently active one during expansion).
I agree. Initially, both stmt and expressions is set to NULL block.
Whenever we update the stmt's block, we should also update the
expression's block, otherwise they will be inconsistent.
No, we should not. Basically the thing to keep in mind is that the
location/block of expressions after gimplification is supposed to be
meaningless. The gimple statements are the anchors for locations.
Everything should be made to work in this model, not by tacking locations
to expression. But see below...
I don't see how we could end up generating too many block location
DIEs because of this.
For the unittest I added, all the assembly code guarded by if are
should be within one lexical block, which is lock:
LBB1
{
asm1
asm2
asm3
asm4
}
However, because some expression are with NULL lexical block, these
assembly codes are separated by several discontinual lexical block
which is like:
LBB1
{
asm1
}
asm2
LBB2
{
asm3
}
asm4
Hmm, I asked about specifics for this testcase. Let me find out myself...
You're talking about this situation:
.LBB4:
# code for line 20
A
.LBE4:
.loc 1 22 0
movq-16(%rbp), %rax # key, tmp82
.LBB5:
# code for line 22/23
.LBE5:
movq-16(%rbp), %rax # key, tmp83
.LBB6:
.LBE6
where the reload of 'key' is splitting the lexical block? This comes from
_10 = key_3-rulesToParseHdl;
Where the MEM_REF of *key_3 does have a location (hence locus), but no
block associated:
(gdb) p expand_location(((tree)0x762252d0)-exp.locus)
$12 = {file = 0x7fffe38e blabla.C, line = 22, column = 29, data =
0x0, sysp = false}
whereas the statement has the same location, but is associated with a
block:
(gdb) p expand_location(curr_location )
$14 = {file = 0x7fffe38e blabla.C, line = 22, column = 29,
data = 0x760cf410, sysp = false}
This inconsistency is indeed generated by lower_stmt, which inserts
the block into this locator for the statements, but not for operands.
The issue is that tree-RTL expand listens to what's stored in the
EXPR_LOCATION, instead of taking the location info from curr_location
(which itself comes from the currently expanded gimple statement).
The patch/hack below solves this, and probably also your problem. It
might introduce others, but I think we should work towards making this
patch possible. I.e. follow the above layed out model, that after
gimplification locators are _only_ in gimple statements, nowhere else
(though I'm not sure what to do about constructs that don't generate
statements, like simple global data constructors).
I'm not sure how realistic reaching this goal for 4.8 is. If it isn't
then I fear something along the lines of your hack is necessary.
Ciao,
Michael.
-
Index: expr.c
===
--- expr.c (revision 192809)
+++ expr.c (working copy)
@@ -5030,7 +5030,7 @@ store_expr (tree exp, rtx target, int ca
{
rtx temp;
rtx alt_rtl = NULL_RTX;
- location_t loc = EXPR_LOCATION (exp);
+ location_t loc = curr_insn_location ();
if (VOID_TYPE_P (TREE_TYPE (exp)))
{
Re: RFA: patch to fix PR55116
On 12-10-29 12:21 PM, Richard Sandiford wrote: Vladimir Makarov vmaka...@redhat.com writes: H.J. in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55116 reported an interesting address (and:DI (subreg:DI (plus:SI (ashift:SI (reg:SI 96 [ glob_vol_int.22 ]) (const_int 2 [0x2])) (symbol_ref:SI (glob_vol_int_arr) var_decl 0x703c2720 glob_vol_int_arr)) 0) (const_int 4294967295 [0x])) which can not be correctly extracted. Here `and' with `subreg' behaves as an address mutation. The following patch fixes the problem. Ok to commit, Richard? Heh, I wondered if subregs might still be used like that, and was almost tempted to add them just in case. I think this particular case is really a failed canonicalisation and that: (and:DI (subreg:DI (foo:SI ...) 0) (const_int 0x)) ought to be: (zero_extend:DI (foo:SI ...)) Yes, that was my thought too. But I know I've approved MIPS patches to accept (and:DI ... (const_int 0x)) as an alternative. Index: rtlanal.c === --- rtlanal.c (revision 192942) +++ rtlanal.c (working copy) @@ -5459,6 +5459,11 @@ strip_address_mutations (rtx *loc, enum else if (code == AND CONST_INT_P (XEXP (*loc, 1))) /* (and ... (const_int -X)) is used to align to X bytes. */ loc = XEXP (*loc, 0); + else if (code == SUBREG + ! REG_P (XEXP (*loc, 0)) ! MEM_P (XEXP (*loc, 0))) + /* (subreg (operator ...) ...) usually inside and is used for + mode conversion too. */ + loc = XEXP (*loc, 0); I think the condition should be: else if (code == SUBREG !OBJECT_P (SUBREG_REG (*loc)) subreg_lowpart (*loc)) OK with that change, if it works. Yes, it works. I've submitted the following patch. Index: rtlanal.c === --- rtlanal.c (revision 192942) +++ rtlanal.c (working copy) @@ -5459,6 +5459,12 @@ strip_address_mutations (rtx *loc, enum else if (code == AND CONST_INT_P (XEXP (*loc, 1))) /* (and ... (const_int -X)) is used to align to X bytes. */ loc = XEXP (*loc, 0); + else if (code == SUBREG +!OBJECT_P (SUBREG_REG (*loc)) +subreg_lowpart_p (*loc)) + /* (subreg (operator ...) ...) inside and is used for mode + conversion too. */ + loc = XEXP (*loc, 0); else return loc; if (outer_code)
Re: RFA: patch to fix PR55116
On 12-10-29 12:30 PM, Richard Sandiford wrote: Sorry, forgot a line: i.e. please use SUBREG_REG rather than XEXP. I've just fixed it.
[PATCH, i386]: Simplify AND-extended address decomposition
Hello!
Since we use simplify_gen_subreg on address part, we can use it for
DImode subreg of SImode address as well. simplify_gen_subreg knows how
to strip subreg.
2012-10-29 Uros Bizjak ubiz...@gmail.com
* config/i386/i386.c (ix86_decompose_address): Use simplify_gen_subreg
for all addresses, zero-extended with AND.
Tested on x86_64-pc-linux-gnu {,-m32}, committed to mainline SVN.
Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 192933)
+++ config/i386/i386.c (working copy)
@@ -11810,23 +11810,11 @@ ix86_decompose_address (rtx addr, struct ix86_addr
else if (GET_CODE (addr) == AND
const_32bit_mask (XEXP (addr, 1), DImode))
{
- addr = XEXP (addr, 0);
+ addr = simplify_gen_subreg (SImode, XEXP (addr, 0), DImode, 0);
+ if (addr == NULL_RTX)
+ return 0;
- /* Adjust SUBREGs. */
- if (GET_CODE (addr) == SUBREG
- GET_MODE (SUBREG_REG (addr)) == SImode)
- {
- addr = SUBREG_REG (addr);
- if (CONST_INT_P (addr))
- return 0;
- }
- else if (GET_MODE (addr) == DImode)
- {
- addr = simplify_gen_subreg (SImode, addr, DImode, 0);
- if (addr == NULL_RTX)
- return 0;
- }
- else if (GET_MODE (addr) != VOIDmode)
+ if (CONST_INT_P (addr))
return 0;
}
}
Re: [PATCH] Fix debug info for expr and jump stmt
On Mon, Oct 29, 2012 at 9:10 AM, Richard Biener
richard.guent...@gmail.com wrote:
On Mon, Oct 29, 2012 at 4:25 PM, Dehao Chen de...@google.com wrote:
On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz m...@suse.de wrote:
Hi,
On Mon, 29 Oct 2012, Richard Biener wrote:
Well, you merely moved the bogus code to gimple-low.c. It is bogus
because you unconditionally overwrite TREE_BLOCK of all operands (and all
Emm, then in gimple-low.c, we should probably not unconditionally
overwrite gimple_block for stmts too?
gimple stmts have no block before gimple-low.
operands operands) with the statements block. I assume the unit-test you
added shows the problem you were trying to fix?
From the scan-assembler-no directive I'm not really sure what exactly the
problem is you're seeing. Can you try to describe it with words for the
example code? Which operands has no tree-block where it should have one,
or which one has the wrong tree-block?
trees without block could be an issue when we extract them to some other
statement (then without block), and move that to some random place.
Even then it would be either the frontends job to set the block, or the
the job of the pass that introduced the new expression, when there is a
clearly sane candidate for the block.
Please correct me if I'm wrong: front-end does not set blocks for
either stmt/expr. lower_stmt is the first place that block is set for
stmt, thus I think it should also be the first place to set block for
expr.
No, only the block on the gimple stmt matters for gimple. You should not
need to touch the operands block.
But the only issue should be that the stmt/expressions effective block
becomes a different one (the currently active one during expansion).
I agree. Initially, both stmt and expressions is set to NULL block.
Whenever we update the stmt's block, we should also update the
expression's block, otherwise they will be inconsistent.
I don't think so.
Hi, Richard,
Can you share some insights why you don't think we want to make stmt
and expr's block consistent?
Yep.
I don't see how we could end up generating too many block location
DIEs because of this.
For the unittest I added, all the assembly code guarded by if are
should be within one lexical block, which is lock:
LBB1
{
asm1
asm2
asm3
asm4
}
However, because some expression are with NULL lexical block, these
assembly codes are separated by several discontinual lexical block
which is like:
LBB1
{
asm1
}
asm2
asm2 should have gone into the currently open scope (similiar for what
we do for line number information).
In location_block patch, we changed this not to fall into the
currently open scope, because we want every insn to have correct
location/block with it. This is one of the reasons why we introduced
the location_block patch: it makes it easy to update block for
stmt/expr/phi_arg thus keeping a consistent location/block pair for
all instructions becomes possible.
Yes, let location/block fall into the currently open scope/location
was a workaround before, but if we are able to make the location/block
pair consistent all through compilation, we should do that, right?
Thanks,
Dehao
LBB2
{
asm3
}
asm4
And even if, I don't see what's the _problem_ with too many block
locations, except bloat.
Yes, bloat is the major problem.
Thanks,
Dehao
Ciao,
Michael.
Parts 3 and 4 to the vxworks/fixincludes patches
The first two patches I've applied. The remaining two are needed to fully enable building the VxWorks flavor of GCC, but those bits affect parts outside of fixincludes and there is some breakage somewhere. All evidence seems to me to show fixincludes still doing its thing correctly, but somewhere along the line the build becomes confused and unable to find stdarg.h: echo timestamp stmp-int-hdrs g++ -c -g -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables \ -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute \ -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common \ -DHAVE_CONFIG_H -I. -Ilto -I../.././gcc -I../.././gcc/lto -I../.././gcc/../include \ -I../.././gcc/../libcpp/include -I../.././gcc/../libdecnumber \ -I../.././gcc/../libdecnumber/bid -I../libdecnumber -I../.././gcc/../libbacktrace \ ../.././gcc/lto/lto-lang.c -o lto/lto-lang.o In file included from ../.././gcc/lto/lto-lang.c:22:0: ../.././gcc/system.h:28:20: fatal error: stdarg.h: No such file or directory #include stdarg.h ^ compilation terminated. I have not been able to run down the cause. Until I've found it, I'm holding back on the bits that change stuff outside of fixincludes: gcc/gcov-io.c libstdc++-v3/config/os/vxworks/os_defines.h configure.ac I hope someone knows what this is.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
On Mon, Oct 29, 2012 at 12:29 PM, Arnaud Charlet char...@adacore.com wrote: Committed on trunk now. Sorry for the breakage, my build was incremental and missed this missing part. Great. Thanks for the quick fix! Diego.
Re: [PATCH] Fix debug info for expr and jump stmt
Hi, Micheal,
Thanks for explaining the design principle of debug info with gimple,
now I can understand your concerns. And thanks for providing the
patch.
However, in some places after gimplification (e.g. tree-inline.c), we
still updates the block/location info. And EXPR_LOCATION is still used
widely after gimplification. Do you mean that in the long run, we'd
want to remove all these?
Thanks,
Dehao
On Mon, Oct 29, 2012 at 9:49 AM, Dehao Chen de...@google.com wrote:
On Mon, Oct 29, 2012 at 9:10 AM, Richard Biener
richard.guent...@gmail.com wrote:
On Mon, Oct 29, 2012 at 4:25 PM, Dehao Chen de...@google.com wrote:
On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz m...@suse.de wrote:
Hi,
On Mon, 29 Oct 2012, Richard Biener wrote:
Well, you merely moved the bogus code to gimple-low.c. It is bogus
because you unconditionally overwrite TREE_BLOCK of all operands (and
all
Emm, then in gimple-low.c, we should probably not unconditionally
overwrite gimple_block for stmts too?
gimple stmts have no block before gimple-low.
operands operands) with the statements block. I assume the unit-test
you
added shows the problem you were trying to fix?
From the scan-assembler-no directive I'm not really sure what exactly
the
problem is you're seeing. Can you try to describe it with words for the
example code? Which operands has no tree-block where it should have
one,
or which one has the wrong tree-block?
trees without block could be an issue when we extract them to some other
statement (then without block), and move that to some random place.
Even then it would be either the frontends job to set the block, or the
the job of the pass that introduced the new expression, when there is a
clearly sane candidate for the block.
Please correct me if I'm wrong: front-end does not set blocks for
either stmt/expr. lower_stmt is the first place that block is set for
stmt, thus I think it should also be the first place to set block for
expr.
No, only the block on the gimple stmt matters for gimple. You should not
need to touch the operands block.
But the only issue should be that the stmt/expressions effective block
becomes a different one (the currently active one during expansion).
I agree. Initially, both stmt and expressions is set to NULL block.
Whenever we update the stmt's block, we should also update the
expression's block, otherwise they will be inconsistent.
I don't think so.
Hi, Richard,
Can you share some insights why you don't think we want to make stmt
and expr's block consistent?
Yep.
I don't see how we could end up generating too many block location
DIEs because of this.
For the unittest I added, all the assembly code guarded by if are
should be within one lexical block, which is lock:
LBB1
{
asm1
asm2
asm3
asm4
}
However, because some expression are with NULL lexical block, these
assembly codes are separated by several discontinual lexical block
which is like:
LBB1
{
asm1
}
asm2
asm2 should have gone into the currently open scope (similiar for what
we do for line number information).
In location_block patch, we changed this not to fall into the
currently open scope, because we want every insn to have correct
location/block with it. This is one of the reasons why we introduced
the location_block patch: it makes it easy to update block for
stmt/expr/phi_arg thus keeping a consistent location/block pair for
all instructions becomes possible.
Yes, let location/block fall into the currently open scope/location
was a workaround before, but if we are able to make the location/block
pair consistent all through compilation, we should do that, right?
Thanks,
Dehao
LBB2
{
asm3
}
asm4
And even if, I don't see what's the _problem_ with too many block
locations, except bloat.
Yes, bloat is the major problem.
Thanks,
Dehao
Ciao,
Michael.
Re: Ping: [PATCH] Install error handler for out-of-memory when using STL containers
On Oct 29, 2012, at 2:02 AM, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: It looks straightforward to me but I am not convinced that a fortran maintainer should be giving the green light on this :-) Well, seems reasonable to me as well… I'd let Diego or Jason approve it. :-)
Re: RFA: patch to fix PR55116
On 12-10-29 12:21 PM, Richard Sandiford wrote: Vladimir Makarov vmaka...@redhat.com writes: H.J. in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55116 reported an interesting address (and:DI (subreg:DI (plus:SI (ashift:SI (reg:SI 96 [ glob_vol_int.22 ]) (const_int 2 [0x2])) (symbol_ref:SI (glob_vol_int_arr) var_decl 0x703c2720 glob_vol_int_arr)) 0) (const_int 4294967295 [0x])) which can not be correctly extracted. Here `and' with `subreg' behaves as an address mutation. The following patch fixes the problem. Ok to commit, Richard? Heh, I wondered if subregs might still be used like that, and was almost tempted to add them just in case. I think this particular case is really a failed canonicalisation and that: (and:DI (subreg:DI (foo:SI ...) 0) (const_int 0x)) ought to be: (zero_extend:DI (foo:SI ...)) This canonicalisation would make my life much easier. Please note that (and:DI ((foo:DI ...) (const_int 0x))) is also valid address. We have tripped on it a couple of times with x32. Uros.
Re: User directed Function Multiversioning via Function Overloading (issue5752064)
On Mon, Oct 29, 2012 at 5:55 AM, Jan Hubicka hubi...@ucw.cz wrote:
Index: gcc/cgraph.c
===
--- gcc/cgraph.c (revision 192623)
+++ gcc/cgraph.c (working copy)
@@ -132,6 +132,74 @@ static GTY(()) struct cgraph_edge *free_edges;
/* Did procss_same_body_aliases run? */
bool same_body_aliases_done;
+/* Map a cgraph_node to cgraph_function_version_info using this htab.
+ The cgraph_function_version_info has a THIS_NODE field that is the
+ corresponding cgraph_node.. */
+htab_t GTY((param_is (struct cgraph_function_version_info *)))
+ cgraph_fnver_htab = NULL;
I think you want declare the htab static and arrange it to be freed after
cgraph construction, so you don't need to take care of nodes being removed
via the hooks.
I will declare the htab static but I want this htab for later
optimizations, like dispatch hoisting. Please see:
http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02285.html for a
description of the optimization. IFUNC based dispatch blocks inlining
of multi-versioned functions and dispatch hoisting will help with
this.
I will make the other changes asap.
Thanks,
-Sri.
OK with this change.
I have few other comments:
+ /* IFUNC resolvers have to be externally visible. */
+ TREE_PUBLIC (decl) = 1;
+ DECL_UNINLINABLE (decl) = 1;
Why the resolvers can not be inlined?
+
+ DECL_EXTERNAL (decl) = 0;
+ DECL_EXTERNAL (dispatch_decl) = 0;
+
+ DECL_CONTEXT (decl) = NULL_TREE;
+ DECL_INITIAL (decl) = make_node (BLOCK);
+ DECL_STATIC_CONSTRUCTOR (decl) = 0;
+ TREE_READONLY (decl) = 0;
+ DECL_PURE_P (decl) = 0;
I think those can be copied from the functions you are resolving. (well as
well
as many attributes and properties)
+
+ if (DECL_COMDAT_GROUP (default_decl))
+{
+ DECL_COMDAT (decl) = DECL_COMDAT (default_decl);
+ make_decl_one_only (decl, DECL_COMDAT_GROUP (default_decl));
+}
+ else if (TREE_PUBLIC (default_decl))
+{
+ /* In this case, each translation unit with a call to this
+ versioned function will put out a resolver. Ensure it
+ is comdat to keep just one copy. */
+ DECL_COMDAT (decl) = 1;
+ make_decl_one_only (decl, DECL_ASSEMBLER_NAME (decl));
+}
+ /* Build result decl and add to function_decl. */
+ t = build_decl (UNKNOWN_LOCATION, RESULT_DECL, NULL_TREE, ptr_type_node);
+ DECL_ARTIFICIAL (t) = 1;
+ DECL_IGNORED_P (t) = 1;
+ DECL_RESULT (decl) = t;
+
+ gimplify_function_tree (decl);
+ push_cfun (DECL_STRUCT_FUNCTION (decl));
+ gimple_register_cfg_hooks ();
+ init_empty_tree_cfg_for_function (DECL_STRUCT_FUNCTION (decl));
+ cfun-curr_properties |=
+(PROP_gimple_lcf | PROP_gimple_leh | PROP_cfg | PROP_ssa
+ | PROP_gimple_any);
+ cfun-curr_properties = 15;
+ new_bb = create_empty_bb (ENTRY_BLOCK_PTR);
+ make_edge (ENTRY_BLOCK_PTR, new_bb, EDGE_FALLTHRU);
+ make_edge (new_bb, EXIT_BLOCK_PTR, 0);
+ *empty_bb = new_bb;
You can simplify this by init_lowered_empty_function.
Honza
Re: Fix estimation of array accesses
ICK ...
Why not sth as simple as
return num_ssa_operands (stmt, SSA_OP_USE);
? a[1][2] and b[2] really have the same cost, variable length
objects have extra SSA operands in ARRAY_REF/COMPONENT_REF for
the size. Thus, stmt cost somehow should reflect the number
of dependent stmts.
So in estimate_num_insns I'd try
int
estimate_num_insns (gimple stmt, eni_weights *weights)
{
unsigned cost, i;
enum gimple_code code = gimple_code (stmt);
tree lhs;
tree rhs;
switch (code)
{
case GIMPLE_ASSIGN:
/* Initialize with the number of SSA uses, one is free. */
cost = num_ssa_operands (stmt, SSA_OP_USE);
if (cost 1)
--cost;
Hi,
this is the udpated patch I am testing after today discussion. I decided to
drop the ipa-inline-analysis changes and do that incrementally. So the patch
now trashes tramp3d performance by increasing need for early-inlining-insns,
but it is not inexpected.
The patch also fixes accounting of addr expr that got previously confused with
a load.
Does this seem sane?
* tree-inline.c (estimate_operator_cost): Return 1 for non-trivial
ADDR_EXPR operations.
(estimate_num_insns): Do not confuse general single rhs with
loads; account cost of non-trivial addr_expr for ASSIGN_EXPR,
GIMPLE_RETURN and GIMPLE_ASM
Hi,
this patch actually do not cause that much of tramp3d fuzz and no differences
in testsuite due to unroling changes
The change are the constants when accounting loads and stores.
Typical store has two SSA uses (one for address and one for value to store).
Of course we lose difference in between array offset and pointer dereference.
Typical load/address expression has one SSA use (for the address)
Bootstrapped/regtested x86_64-linux, OK?
Honza
Index: tree-inline.c
===
--- tree-inline.c (revision 192945)
+++ tree-inline.c (working copy)
@@ -3447,6 +3447,19 @@ estimate_operator_cost (enum tree_code c
if (TREE_CODE (op2) != INTEGER_CST)
return weights-div_mod_cost;
return 1;
+case ADDR_EXPR:
+ {
+tree addr_base;
+HOST_WIDE_INT addr_offset;
+
+ addr_base = get_addr_base_and_unit_offset (TREE_OPERAND (op1, 0),
+ addr_offset);
+ /* If the offset is variable or with non-zero offset, return 2. */
+ if (!addr_base || addr_offset != 0 || TREE_CODE (addr_base) != MEM_REF
+ || !integer_zerop (TREE_OPERAND (addr_base, 1)))
+ return 1;
+ }
+ return 0;
default:
/* We expect a copy assignment with no operator. */
@@ -3512,14 +3525,24 @@ estimate_num_insns (gimple stmt, eni_wei
lhs = gimple_assign_lhs (stmt);
rhs = gimple_assign_rhs1 (stmt);
- if (is_gimple_reg (lhs))
- cost = 0;
- else
+ /* Store. */
+ if (gimple_vdef (stmt))
cost = estimate_move_cost (TREE_TYPE (lhs));
+ else
+ cost = 0;
- if (!is_gimple_reg (rhs) !is_gimple_min_invariant (rhs))
+ /* Load. */
+ if (gimple_vuse (stmt))
cost += estimate_move_cost (TREE_TYPE (rhs));
+ /* Stores, loads and address expressions may have variable array
+references in them. Account these. */
+ if (gimple_vdef (stmt))
+ cost += MAX (0, (int)num_ssa_operands (stmt, SSA_OP_USE) - 2);
+ else if (gimple_vuse (stmt)
+ || gimple_assign_rhs_code (stmt) == ADDR_EXPR)
+ cost += MAX (0, (int)num_ssa_operands (stmt, SSA_OP_USE) - 1);
+
cost += estimate_operator_cost (gimple_assign_rhs_code (stmt), weights,
gimple_assign_rhs1 (stmt),
get_gimple_rhs_class
(gimple_assign_rhs_code (stmt))
@@ -3596,6 +3619,7 @@ estimate_num_insns (gimple stmt, eni_wei
case GIMPLE_RETURN:
return weights-return_cost;
++ MAX (0, (int)num_ssa_operands (stmt, SSA_OP_USE) - 1);
case GIMPLE_GOTO:
case GIMPLE_LABEL:
@@ -3606,7 +3630,10 @@ estimate_num_insns (gimple stmt, eni_wei
return 0;
case GIMPLE_ASM:
- return asm_str_count (gimple_asm_string (stmt));
+ return (asm_str_count (gimple_asm_string (stmt))
+ /* Account also possible non-trivial addr_exprs
+in the arguments. */
+ + num_ssa_operands (stmt, SSA_OP_USE));
case GIMPLE_RESX:
/* This is either going to be an external function call with one
GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon
Status == I'd like to close the stage 1 phase of GCC 4.8 development on Monday, November 5th. If you have still patches for new features you'd like to see in GCC 4.8, please post them for review soon. Patches posted before the freeze, but reviewed shortly after the freeze, may still go in, further changes should be just bugfixes and documentation fixes. Quality Data Priority # Change from Last Report --- --- P1 23 + 23 P2 77 + 8 P3 85 + 84 --- --- Total 185 +115 Previous Report === http://gcc.gnu.org/ml/gcc/2012-03/msg00011.html The next report will be sent by me again, announcing end of stage 1.
Re: [PATCH] Fix libbacktrace on 32-bit sparc
From: Ian Lance Taylor i...@google.com Date: Mon, 29 Oct 2012 08:43:42 -0700 This patch should fix it. Bootstrapped and ran libbacktrace testsuite on x86_64-unknown-linux-gnu. Committed to mainline. I can tell just by looking at your patch that it doesn't fix the problem. It's vec-base + vec-size that's not aligned, rather than the length. All your change does it align the length term. You have to align the vec-size term in the pointer computation. Actually you need to align both. First you have to align vec-size to an 8 byte boundary. Then you have to align the remaining length that results to an 8 byte boundary as well. Can you post a test patch for me this time, instead of just committing a change which hasn't been tested to fix the problem? I'd very much appreciate that. Thanks.
Re: GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon
From: Jakub Jelinek ja...@redhat.com Date: Mon, 29 Oct 2012 18:56:42 +0100 I'd like to close the stage 1 phase of GCC 4.8 development on Monday, November 5th. If you have still patches for new features you'd like to see in GCC 4.8, please post them for review soon. Patches posted before the freeze, but reviewed shortly after the freeze, may still go in, further changes should be just bugfixes and documentation fixes. I'd like to get the Sparc cbcond stuff in (3 revisions posted) which is waiting for Eric B. to do some Solaris specific work. I'd also like to enable LRA for at least 32-bit sparc, even if I can't find the time to work on auditing 64-bit completely.
Re: Minor update for inlining for code size
Bootstrapped/regtested x86_64-linux, will commit it shortly. Honza * gcc.dg/ipa/inline-6.c: New testcase. * ipa-inline.c (want_inline_function_called_once_p): Rename to ... (want_inline_function_to_all_callers_p): check also functions with multiple callers. (ipa_inline): Handle inlining for size into multiple callers. The entry for the testcase must go into testsuite/ChangeLog. -- Eric Botcazou
Re: [PATCH] Fix debug info for expr and jump stmt
On Mon, Oct 29, 2012 at 6:07 PM, Dehao Chen de...@google.com wrote:
Hi, Micheal,
Thanks for explaining the design principle of debug info with gimple,
now I can understand your concerns. And thanks for providing the
patch.
However, in some places after gimplification (e.g. tree-inline.c), we
still updates the block/location info. And EXPR_LOCATION is still used
widely after gimplification. Do you mean that in the long run, we'd
want to remove all these?
Yes. Unfortunately many of the core inlining routines are also used by
the C++ frontend for template instantiation...
Richard.
Thanks
Dehao
On Mon, Oct 29, 2012 at 9:49 AM, Dehao Chen de...@google.com wrote:
On Mon, Oct 29, 2012 at 9:10 AM, Richard Biener
richard.guent...@gmail.com wrote:
On Mon, Oct 29, 2012 at 4:25 PM, Dehao Chen de...@google.com wrote:
On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz m...@suse.de wrote:
Hi,
On Mon, 29 Oct 2012, Richard Biener wrote:
Well, you merely moved the bogus code to gimple-low.c. It is bogus
because you unconditionally overwrite TREE_BLOCK of all operands (and
all
Emm, then in gimple-low.c, we should probably not unconditionally
overwrite gimple_block for stmts too?
gimple stmts have no block before gimple-low.
operands operands) with the statements block. I assume the unit-test
you
added shows the problem you were trying to fix?
From the scan-assembler-no directive I'm not really sure what exactly
the
problem is you're seeing. Can you try to describe it with words for
the
example code? Which operands has no tree-block where it should have
one,
or which one has the wrong tree-block?
trees without block could be an issue when we extract them to some other
statement (then without block), and move that to some random place.
Even then it would be either the frontends job to set the block, or the
the job of the pass that introduced the new expression, when there is a
clearly sane candidate for the block.
Please correct me if I'm wrong: front-end does not set blocks for
either stmt/expr. lower_stmt is the first place that block is set for
stmt, thus I think it should also be the first place to set block for
expr.
No, only the block on the gimple stmt matters for gimple. You should not
need to touch the operands block.
But the only issue should be that the stmt/expressions effective block
becomes a different one (the currently active one during expansion).
I agree. Initially, both stmt and expressions is set to NULL block.
Whenever we update the stmt's block, we should also update the
expression's block, otherwise they will be inconsistent.
I don't think so.
Hi, Richard,
Can you share some insights why you don't think we want to make stmt
and expr's block consistent?
Yep.
I don't see how we could end up generating too many block location
DIEs because of this.
For the unittest I added, all the assembly code guarded by if are
should be within one lexical block, which is lock:
LBB1
{
asm1
asm2
asm3
asm4
}
However, because some expression are with NULL lexical block, these
assembly codes are separated by several discontinual lexical block
which is like:
LBB1
{
asm1
}
asm2
asm2 should have gone into the currently open scope (similiar for what
we do for line number information).
In location_block patch, we changed this not to fall into the
currently open scope, because we want every insn to have correct
location/block with it. This is one of the reasons why we introduced
the location_block patch: it makes it easy to update block for
stmt/expr/phi_arg thus keeping a consistent location/block pair for
all instructions becomes possible.
Yes, let location/block fall into the currently open scope/location
was a workaround before, but if we are able to make the location/block
pair consistent all through compilation, we should do that, right?
Thanks,
Dehao
LBB2
{
asm3
}
asm4
And even if, I don't see what's the _problem_ with too many block
locations, except bloat.
Yes, bloat is the major problem.
Thanks,
Dehao
Ciao,
Michael.
Re: [PATCH] Fix libbacktrace on 32-bit sparc
On Mon, Oct 29, 2012 at 11:03 AM, David Miller da...@davemloft.net wrote: From: Ian Lance Taylor i...@google.com Date: Mon, 29 Oct 2012 08:43:42 -0700 This patch should fix it. Bootstrapped and ran libbacktrace testsuite on x86_64-unknown-linux-gnu. Committed to mainline. I can tell just by looking at your patch that it doesn't fix the problem. It's vec-base + vec-size that's not aligned, rather than the length. All your change does it align the length term. You have to align the vec-size term in the pointer computation. I did. I changed it from vec-base + vec-size. Now it is vec-base + size, where size is the aligned version of vec-size. I know that vec-base is aligned. Now that size is aligned, I know that vec-base + size is aligned. First you have to align vec-size to an 8 byte boundary. Then you have to align the remaining length that results to an 8 byte boundary as well. Yes, the patch does both of those things. Can you post a test patch for me this time, instead of just committing a change which hasn't been tested to fix the problem? I'd very much appreciate that. Sure, if you can confirm that this patch fails, I will take another look. Ian
Re: [PATCH] Fix libbacktrace on 32-bit sparc
From: Ian Lance Taylor i...@google.com Date: Mon, 29 Oct 2012 11:13:19 -0700 Sure, if you can confirm that this patch fails, I will take another look. It fails. Same problem, SIGBUS.
Re: [PATCH] Fix libbacktrace on 32-bit sparc
From: Ian Lance Taylor i...@google.com Date: Mon, 29 Oct 2012 11:13:19 -0700 I changed it from vec-base + vec-size. Now it is vec-base + size, where size is the aligned version of vec-size. It is not. You don't modify 'size' at all.
Re: [PATCH GCC]Fix test case failure reported in PR54989
On 10/26/2012 10:14 PM, Bin Cheng wrote: Actually I am going to submit a patch monitoring the change of register pressure accurately as we discussed before. With that patch, expressions will be hoisted anyway if it can decrease register pressure thus this test case won't fail anymore. So I am wondering whether I should check in this patch or just waiting for that patch to fix this failure? What's your opinion? I'd go ahead and get in the fix for this failure as we would prefer as few unexpected failures as possible. jeff
Re: GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon
I'd like to get the Sparc cbcond stuff in (3 revisions posted) which is waiting for Eric B. to do some Solaris specific work. I'd also like to enable LRA for at least 32-bit sparc, even if I can't find the time to work on auditing 64-bit completely. End of stage #1 isn't a hard limit for architecture-specific patches, so we need not make a decision about LRA immediately. I don't think we want to half enable it though, so it's all or nothing. -- Eric Botcazou
Re: [PATCH] Fix libbacktrace on 32-bit sparc
On Mon, Oct 29, 2012 at 11:15 AM, David Miller da...@davemloft.net wrote: From: Ian Lance Taylor i...@google.com Date: Mon, 29 Oct 2012 11:13:19 -0700 I changed it from vec-base + vec-size. Now it is vec-base + size, where size is the aligned version of vec-size. It is not. You don't modify 'size' at all. Ah, yes, sorry. Could you test this patch? Ian Index: mmap.c === --- mmap.c (revision 192945) +++ mmap.c (working copy) @@ -240,7 +240,8 @@ backtrace_vector_release (struct backtra aligned = (size + 7) ~ (size_t) 7; alc -= aligned - size; - backtrace_free (state, (char *) vec-base + size, alc, error_callback, data); + backtrace_free (state, (char *) vec-base + aligned, alc, + error_callback, data); vec-alc = 0; return 1; }
[patch] move GIMPLE_TRANSACTION expansion to tmmark pass
What was supposed to be a simple fix, turned out to be a nightmare...
I am working on adding an uninstrumented code path for TM, in
expectation of hardware TM. This means that we eventually want to
generate something like this:
prop = _ITM_beginTransaction(PR_multiwayCode);
if (prop PR_UNINSTRUMENTEDCODE)
global = 55;
else
_ITM_WU4 (global, 55);
_ITM_commitTransaction ();
Since reading prop and duplicating BB's must happen at the TMMARK pass
(instrumentation pass), we must move the expansion of
GIMPLE_TRANSACTION - _ITM_beginTransaction() into the TMMARK pass,
instead of a later pass like is currently being done.
Moving this, caused all sorts of grief, because I found that the
abnormal edges for each TM built-in, have been incorrectly generated
since day one. I have no idea, how things worked before...
Every TM built-in must have an abnormal edge back to the outer-most
transaction. As it currently stands, TM built-ins have an edge to the
current transaction, which means that for nested transactions, we end up
with incorrect edges to every transaction.
Transaction cancel statements, OTOH, must _also_ have an edge to the
inner transaction-- the transaction to which the cancel appears in.
And finally, __transaction_cancel[[outer]] statements must only have one
abnormal edge to the transaction marked as outer by the front-end
(__transaction_atomic[[outer]]).
I had to fix all this, as part of the work moving the transaction
expansion into the TMMARK pass. There are also various cleanups that
seemed sensible while I was re-arranging things, so my apologies for the
plethora of fixes.
I have not added further tests, since the present testsuite triggers the
problems I found.
Finally, I adapted various tests, because things now happen in other passes.
OK, pending one more round of testing?
p.s. You'll hate tm_region_for_stmt*(), but (a) it's only called once
for every __transaction_cancel, which is rare (b) I was really tired
:)). If you hate it more than I do, I can optimize it later.
+ * trans-mem.c (tm_log_emit_save_or_restores): Remove FALLTHRU_EDGE
+ parameter and calculate locally.
+ (struct tm_region): Add new fields:
+ original_transaction_was_outer, transaction_label, tm_state.
+ (tm_region_init_0): Initialize new tm_region fields.
+ (expand_assign_tm): Add comments.
+ (expand_transaction): Reorganize to be called in tmmark pass
+ instead of the tmedge pass.
+ (generate_tm_state): New.
+ (execute_tm_mark): Generate TM_STATE temporaries. Expand gimple
+ transactions into calls into the run-time.
+ (tm_region_for_stmt_1): New.
+ (tm_region_for_stmt): New.
+ (make_tm_edge): Point abnormal TM built-in edges to the
outermost
+ transaction. Point abnormal TM_ABORT edges appropriately.
+ (tmedge_wire_transaction_restarts): Abstract code previously in
+ expand_transaction to wire abnormal edges.
+ (expand_regions_1): Rewrite to use callback.
+ (expand_regions): Same.
+ (execute_tm_edges): Call expand_regions with new callback.
+ * trans-mem.h (PR_MULTIWAYCODE): New.
+ * tree-cfg.c (gimple_debug_cfg): Update comment.
+
+
testsuite/
+ * gcc.dg/tm/debug-1.c: Scan for _ITM_beginTransaction.
+ * gcc.dg/tm/irrevocable-3.c: Scan for doesGoIrrevocable.
+ * gcc.dg/tm/irrevocable-4.c: Scan for hasNoIrrevocable.
+ * gcc.dg/tm/props-4.c: Do not scan for LABEL.
+ * gcc.dg/tm/memopt-11.c: Adjust scan text.
+ * gcc.dg/tm/memopt-10.c: Adjust scan text.
diff --git a/gcc/testsuite/gcc.dg/tm/debug-1.c
b/gcc/testsuite/gcc.dg/tm/debug-1.c
index fae5d6b..01acfae 100644
--- a/gcc/testsuite/gcc.dg/tm/debug-1.c
+++ b/gcc/testsuite/gcc.dg/tm/debug-1.c
@@ -20,7 +20,7 @@ main() {
}
/* { dg-final { scan-tree-dump-times : 13:.*b = 9898 1 tmmark } } */
-/* { dg-final { scan-tree-dump-times : 14:.*__transaction 1 tmmark } } */
+/* { dg-final { scan-tree-dump-times : 14:.*_ITM_beginTransaction 1 tmmark
} } */
/* { dg-final { scan-tree-dump-times : 15:.*ITM_WU. \\(z 1 tmmark } } */
/* { dg-final { scan-tree-dump-times : 16:.*ITM_WU. \\(a 1 tmmark } } */
/* { dg-final { cleanup-tree-dump tmmark } } */
diff --git a/gcc/testsuite/gcc.dg/tm/irrevocable-3.c
b/gcc/testsuite/gcc.dg/tm/irrevocable-3.c
index c085479..fdf3e52 100644
--- a/gcc/testsuite/gcc.dg/tm/irrevocable-3.c
+++ b/gcc/testsuite/gcc.dg/tm/irrevocable-3.c
@@ -10,5 +10,5 @@ foo()
}
}
-/* { dg-final { scan-tree-dump-times GTMA_MAY_ENTER_IRREVOCABLE 1 tmmark }
} */
+/* { dg-final { scan-tree-dump-times doesGoIrrevocable 1 tmmark } } */
/* { dg-final { cleanup-tree-dump tmmark } } */
diff --git a/gcc/testsuite/gcc.dg/tm/irrevocable-4.c
Re: [PATCH] Fix libbacktrace on 32-bit sparc
From: Ian Lance Taylor i...@google.com Date: Mon, 29 Oct 2012 11:31:33 -0700 On Mon, Oct 29, 2012 at 11:15 AM, David Miller da...@davemloft.net wrote: From: Ian Lance Taylor i...@google.com Date: Mon, 29 Oct 2012 11:13:19 -0700 I changed it from vec-base + vec-size. Now it is vec-base + size, where size is the aligned version of vec-size. It is not. You don't modify 'size' at all. Ah, yes, sorry. Could you test this patch? Will do, hopefully before ConEd turns my power off.
Re: GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon
From: Eric Botcazou ebotca...@adacore.com Date: Mon, 29 Oct 2012 20:25:15 +0100 I'd like to get the Sparc cbcond stuff in (3 revisions posted) which is waiting for Eric B. to do some Solaris specific work. I'd also like to enable LRA for at least 32-bit sparc, even if I can't find the time to work on auditing 64-bit completely. End of stage #1 isn't a hard limit for architecture-specific patches, so we need not make a decision about LRA immediately. I don't think we want to half enable it though, so it's all or nothing. Upon further consideration, agreed. I'll only turn this on if I can get the whole backend working. FWIW, I think we should consider delaying stage1 for another reason. A large number of North American developers are about to be hit by a major natural disaster, and may be without power for weeks.
Re: [PATCH] Fix libbacktrace on 32-bit sparc
From: David Miller da...@davemloft.net Date: Mon, 29 Oct 2012 14:33:33 -0400 (EDT) From: Ian Lance Taylor i...@google.com Date: Mon, 29 Oct 2012 11:31:33 -0700 On Mon, Oct 29, 2012 at 11:15 AM, David Miller da...@davemloft.net wrote: From: Ian Lance Taylor i...@google.com Date: Mon, 29 Oct 2012 11:13:19 -0700 I changed it from vec-base + vec-size. Now it is vec-base + size, where size is the aligned version of vec-size. It is not. You don't modify 'size' at all. Ah, yes, sorry. Could you test this patch? Will do, hopefully before ConEd turns my power off. Yep, this works, thanks!
Re: [PATCH] Fix libbacktrace on 32-bit sparc
On Mon, Oct 29, 2012 at 11:40 AM, David Miller da...@davemloft.net wrote: From: David Miller da...@davemloft.net Date: Mon, 29 Oct 2012 14:33:33 -0400 (EDT) From: Ian Lance Taylor i...@google.com Date: Mon, 29 Oct 2012 11:31:33 -0700 On Mon, Oct 29, 2012 at 11:15 AM, David Miller da...@davemloft.net wrote: From: Ian Lance Taylor i...@google.com Date: Mon, 29 Oct 2012 11:13:19 -0700 I changed it from vec-base + vec-size. Now it is vec-base + size, where size is the aligned version of vec-size. It is not. You don't modify 'size' at all. Ah, yes, sorry. Could you test this patch? Will do, hopefully before ConEd turns my power off. Yep, this works, thanks! Great, thanks for testing it! Committed to mainline. Ian 2012-10-29 Ian Lance Taylor i...@google.com * mmap.c (backtrace_vector_release): Correct last patch: add aligned, not size.
