Re: [PATCH][compare-elim] Merge zero-comparisons with normal ops

[Eric Botcazou]

> This looks good.  OK for the trunk.

FWIW I disagree.  The patch completely shuns the existing implementation of 
the pass, which is based on a forward scan within basic blocks to identify the 
various interesting instructions and record them, and uses full-blown def-use 
and use-def chains instead, which are much more costly to compute.  It's not 
clear to me why the existing implementation couldn't have been extended.

The result is that, for targets for which the pass was initially written, i.e. 
targets for which most (all) arithmetic instructions clobber the flags, the 
pass will be slower for absolutely no benefits, as the existing implementation 
would already have caught all the interesting cases.

So it's again a case of a generic change made for a specific target without 
consideration for other, admittedly less mainstream, targets...

-- 
Eric Botcazou

[committed] Fix pr81423.c testcase (PR rtl-optimization/81423)

[Jakub Jelinek]

Hi!

The original C++ testcase has been transcribed into C and during
that process a UB has been introduced, original had
unsigned(5677365550390624949LL - ll) - (ull1 > 0)
while what has been committed has
(5677365550390624949L - ll) - (ull1 > 0)
I've also changed all L suffixed constants to LL for consistency with
ilp32 (the testcase has been written for x86_64 lp64 apparently)
and added better check that int is 32-bit and long long 64-bit.

Tested on x86_64-linux and i686-linux and verified that using Jul 14th
cc1 still fails on lp64, committed to trunk as obvious.

2017-10-14  Jakub Jelinek  

PR rtl-optimization/81423
* gcc.c-torture/execute/pr81423.c (foo): Add missing cast.  Change L
suffixes to LL.
(main): Punt if either long long isn't 64-bit or int isn't 32-bit.

--- gcc/testsuite/gcc.c-torture/execute/pr81423.c.jj2017-09-01 
09:26:12.0 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr81423.c   2017-10-14 
10:31:25.50882 +0200
@@ -1,3 +1,5 @@
+/* PR rtl-optimization/81423 */
+
 extern void abort (void);
 
 unsigned long long int ll = 0;
@@ -10,11 +12,11 @@ foo (void)
 {
   ll = -5597998501375493990LL;
 
-  ll = (5677365550390624949L - ll) - (ull1 > 0);
+  ll = (unsigned int) (5677365550390624949LL - ll) - (ull1 > 0);
   unsigned long long int ull3;
   ull3 = (unsigned int)
-(2067854353L <<
- (((ll + -2129105131L) ^ 10280750144413668236ULL) -
+(2067854353LL <<
+ (((ll + -2129105131LL) ^ 10280750144413668236ULL) -
   10280750143997242009ULL)) >> ((2873442921854271231ULL | ull2)
- 12098357307243495419ULL);
 
@@ -24,9 +26,10 @@ foo (void)
 int
 main (void)
 {
-  /* We need a long long of exactly 64 bits for this test.  */
-  ll--;
-  if (ll != 0xULL)
+  /* We need a long long of exactly 64 bits and int of exactly 32 bits
+ for this test.  */
+  if (__SIZEOF_LONG_LONG__ * __CHAR_BIT__ != 64
+  || __SIZEOF_INT__ * __CHAR_BIT__ != 32)
 return 0;
 
   ull3 = foo ();

Jakub

[PATCH, i386]: Fix gcc.target/i386/pr71245-?.c scan asm failures

[Uros Bizjak]

Hello!

Now that prerequisite middle-end patch is committed, we can match
atomic FILD/FIST and LDX/STX sequences with a peephole2 pattern that
also include (known) memory blockage instruction.

2017-10-14  Uros Bizjak  

* config/i386/sync.md (FILD_ATOMIC/FIST_ATOMIC FP load peephole2):
Use any_fp_register_operand as operand[3] predicate.  Simplify
equality test for operands[2] and operands[4] memory location.
(LDX_ATOMIC/STX_ATOMIC FP load peephole2): Ditto.
(FILD_ATOMIC/FIST_ATOMIC FP load peephole2 with mem blockage): New.
(LDX_ATOMIC/LDX_ATOMIC FP load peephole2 with mem blockage): Ditto.
(FILD_ATOMIC/FIST_ATOMIC FP store peephole2): Use
any_fp_register_operand as operand[1] predicate.  Simplify
equality test for operands[0] and operands[3] memory location.
(LDX_ATOMIC/STX_ATOMIC FP store peephole2): Ditto.
(FILD_ATOMIC/FIST_ATOMIC FP store peephole2 with mem blockage): New.
(LDX_ATOMIC/LDX_ATOMIC FP storepeephole2 with mem blockage): Ditto.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.
Index: config/i386/sync.md
===
--- config/i386/sync.md (revision 253749)
+++ config/i386/sync.md (working copy)
@@ -219,15 +219,36 @@
(set (match_operand:DI 2 "memory_operand")
(unspec:DI [(match_dup 0)]
   UNSPEC_FIST_ATOMIC))
-   (set (match_operand:DF 3 "fp_register_operand")
+   (set (match_operand:DF 3 "any_fp_register_operand")
(match_operand:DF 4 "memory_operand"))]
   "!TARGET_64BIT
&& peep2_reg_dead_p (2, operands[0])
-   && rtx_equal_p (operands[4], adjust_address_nv (operands[2], DFmode, 0))"
+   && rtx_equal_p (XEXP (operands[4], 0), XEXP (operands[2], 0))"
   [(set (match_dup 3) (match_dup 5))]
   "operands[5] = gen_lowpart (DFmode, operands[1]);")
 
 (define_peephole2
+  [(set (match_operand:DF 0 "fp_register_operand")
+   (unspec:DF [(match_operand:DI 1 "memory_operand")]
+  UNSPEC_FILD_ATOMIC))
+   (set (match_operand:DI 2 "memory_operand")
+   (unspec:DI [(match_dup 0)]
+  UNSPEC_FIST_ATOMIC))
+   (set (mem:BLK (scratch:SI))
+   (unspec:BLK [(mem:BLK (scratch:SI))] UNSPEC_MEMORY_BLOCKAGE))
+   (set (match_operand:DF 3 "any_fp_register_operand")
+   (match_operand:DF 4 "memory_operand"))]
+  "!TARGET_64BIT
+   && peep2_reg_dead_p (2, operands[0])
+   && rtx_equal_p (XEXP (operands[4], 0), XEXP (operands[2], 0))"
+  [(const_int 0)]
+{
+  emit_move_insn (operands[3], gen_lowpart (DFmode, operands[1]));
+  emit_insn (gen_memory_blockage ());
+  DONE;
+})
+
+(define_peephole2
   [(set (match_operand:DF 0 "sse_reg_operand")
(unspec:DF [(match_operand:DI 1 "memory_operand")]
   UNSPEC_LDX_ATOMIC))
@@ -234,14 +255,35 @@
(set (match_operand:DI 2 "memory_operand")
(unspec:DI [(match_dup 0)]
   UNSPEC_STX_ATOMIC))
-   (set (match_operand:DF 3 "fp_register_operand")
+   (set (match_operand:DF 3 "any_fp_register_operand")
(match_operand:DF 4 "memory_operand"))]
   "!TARGET_64BIT
&& peep2_reg_dead_p (2, operands[0])
-   && rtx_equal_p (operands[4], adjust_address_nv (operands[2], DFmode, 0))"
+   && rtx_equal_p (XEXP (operands[4], 0), XEXP (operands[2], 0))"
   [(set (match_dup 3) (match_dup 5))]
   "operands[5] = gen_lowpart (DFmode, operands[1]);")
 
+(define_peephole2
+  [(set (match_operand:DF 0 "sse_reg_operand")
+   (unspec:DF [(match_operand:DI 1 "memory_operand")]
+  UNSPEC_LDX_ATOMIC))
+   (set (match_operand:DI 2 "memory_operand")
+   (unspec:DI [(match_dup 0)]
+  UNSPEC_STX_ATOMIC))
+   (set (mem:BLK (scratch:SI))
+   (unspec:BLK [(mem:BLK (scratch:SI))] UNSPEC_MEMORY_BLOCKAGE))
+   (set (match_operand:DF 3 "any_fp_register_operand")
+   (match_operand:DF 4 "memory_operand"))]
+  "!TARGET_64BIT
+   && peep2_reg_dead_p (2, operands[0])
+   && rtx_equal_p (XEXP (operands[4], 0), XEXP (operands[2], 0))"
+  [(const_int 0)]
+{
+  emit_move_insn (operands[3], gen_lowpart (DFmode, operands[1]));
+  emit_insn (gen_memory_blockage ());
+  DONE;
+})
+
 (define_expand "atomic_store"
   [(set (match_operand:ATOMIC 0 "memory_operand")
(unspec:ATOMIC [(match_operand:ATOMIC 1 "nonimmediate_operand")
@@ -331,7 +373,7 @@
 
 (define_peephole2
   [(set (match_operand:DF 0 "memory_operand")
-   (match_operand:DF 1 "fp_register_operand"))
+   (match_operand:DF 1 "any_fp_register_operand"))
(set (match_operand:DF 2 "fp_register_operand")
(unspec:DF [(match_operand:DI 3 "memory_operand")]
   UNSPEC_FILD_ATOMIC))
@@ -340,13 +382,34 @@
   UNSPEC_FIST_ATOMIC))]
   "!TARGET_64BIT
&& peep2_reg_dead_p (3, operands[2])
-   && rtx_equal_p (operands[0], adjust_address_nv (operands[3], DFmode, 0))"
+   && rtx_equal_p (XEXP (operands[0], 0), XEXP (operands[3], 0))"
   [(set (match_dup 5) (match_dup 1))]
   "operands[5

Re: [PATCH v2, middle-end]: Introduce memory_blockage named insn pattern

[Andrew Pinski]

On Mon, Sep 18, 2017 at 2:06 PM, Uros Bizjak  wrote:
> On Tue, Sep 5, 2017 at 3:50 PM, Uros Bizjak  wrote:
>> Revised patch, incorporates fixes from Alexander's review comments.
>>
>> I removed some implementation details from Alexander's description of
>> memory_blockage named pattern.
>>
>>
>> 2017-09-05  Uros Bizjak  
>>
>> * target-insns.def: Add memory_blockage.
>> * optabs.c (expand_memory_blockage): New function.
>> (expand_asm_memory_barrier): Rename ...
>> (expand_asm_memory_blockage): ... to this.
>> (expand_mem_thread_fence): Call expand_memory_blockage
>> instead of expand_asm_memory_barrier.
>> (expand_mem_singnal_fence): Ditto.
>> (expand_atomic_load): Ditto.
>> (expand_atomic_store): Ditto.
>> * doc/md.texi (Standard Pattern Names For Generation):
>> Document memory_blockage instruction pattern.
>>
>> Bootstrapped and regression tested together with a followup x86 patch
>> on x86_64-linux-gnu {,-m32}.
>>
>> OK for mainline?
>
> PING, original patch at [1].
>
> [1] https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00270.html

This patch broke aarch64-linux-gnu and aarch64-elf building:

./.deps/optabs-query.TPo
/home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/gcc/optabs-query.c
/home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/gcc/optabs.c:
In function ‘void expand_memory_blockage()’:
/home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/gcc/optabs.c:6301:
error: ‘gen_memory_blockage’ was not declared in this scope

Thanks,
Andrew


>
> Uros.

Re: [PATCH v2, middle-end]: Introduce memory_blockage named insn pattern

[Christophe Lyon]

On 14 October 2017 at 12:16, Andrew Pinski  wrote:
> On Mon, Sep 18, 2017 at 2:06 PM, Uros Bizjak  wrote:
>> On Tue, Sep 5, 2017 at 3:50 PM, Uros Bizjak  wrote:
>>> Revised patch, incorporates fixes from Alexander's review comments.
>>>
>>> I removed some implementation details from Alexander's description of
>>> memory_blockage named pattern.
>>>
>>>
>>> 2017-09-05  Uros Bizjak  
>>>
>>> * target-insns.def: Add memory_blockage.
>>> * optabs.c (expand_memory_blockage): New function.
>>> (expand_asm_memory_barrier): Rename ...
>>> (expand_asm_memory_blockage): ... to this.
>>> (expand_mem_thread_fence): Call expand_memory_blockage
>>> instead of expand_asm_memory_barrier.
>>> (expand_mem_singnal_fence): Ditto.
>>> (expand_atomic_load): Ditto.
>>> (expand_atomic_store): Ditto.
>>> * doc/md.texi (Standard Pattern Names For Generation):
>>> Document memory_blockage instruction pattern.
>>>
>>> Bootstrapped and regression tested together with a followup x86 patch
>>> on x86_64-linux-gnu {,-m32}.
>>>
>>> OK for mainline?
>>
>> PING, original patch at [1].
>>
>> [1] https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00270.html
>
> This patch broke aarch64-linux-gnu and aarch64-elf building:
>
> ./.deps/optabs-query.TPo
> /home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/gcc/optabs-query.c
> /home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/gcc/optabs.c:
> In function ‘void expand_memory_blockage()’:
> /home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/gcc/optabs.c:6301:
> error: ‘gen_memory_blockage’ was not declared in this scope
>

Same error for arm.

Christophe

> Thanks,
> Andrew
>
>
>>
>> Uros.

Re: [PATCH v2, middle-end]: Introduce memory_blockage named insn pattern

[Uros Bizjak]

On Sat, Oct 14, 2017 at 12:16 PM, Andrew Pinski  wrote:
> On Mon, Sep 18, 2017 at 2:06 PM, Uros Bizjak  wrote:
>> On Tue, Sep 5, 2017 at 3:50 PM, Uros Bizjak  wrote:
>>> Revised patch, incorporates fixes from Alexander's review comments.
>>>
>>> I removed some implementation details from Alexander's description of
>>> memory_blockage named pattern.
>>>
>>>
>>> 2017-09-05  Uros Bizjak  
>>>
>>> * target-insns.def: Add memory_blockage.
>>> * optabs.c (expand_memory_blockage): New function.
>>> (expand_asm_memory_barrier): Rename ...
>>> (expand_asm_memory_blockage): ... to this.
>>> (expand_mem_thread_fence): Call expand_memory_blockage
>>> instead of expand_asm_memory_barrier.
>>> (expand_mem_singnal_fence): Ditto.
>>> (expand_atomic_load): Ditto.
>>> (expand_atomic_store): Ditto.
>>> * doc/md.texi (Standard Pattern Names For Generation):
>>> Document memory_blockage instruction pattern.
>>>
>>> Bootstrapped and regression tested together with a followup x86 patch
>>> on x86_64-linux-gnu {,-m32}.
>>>
>>> OK for mainline?
>>
>> PING, original patch at [1].
>>
>> [1] https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00270.html
>
> This patch broke aarch64-linux-gnu and aarch64-elf building:
>
> ./.deps/optabs-query.TPo
> /home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/gcc/optabs-query.c
> /home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/gcc/optabs.c:
> In function ‘void expand_memory_blockage()’:
> /home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/gcc/optabs.c:6301:
> error: ‘gen_memory_blockage’ was not declared in this scope

Will fix ASAP:

Index: optabs.c
===
--- optabs.c(revision 253750)
+++ optabs.c(working copy)
@@ -6298,7 +6298,7 @@ static void
 expand_memory_blockage (void)
 {
   if (targetm.have_memory_blockage)
-emit_insn (gen_memory_blockage ());
+emit_insn (targetm.gen_memory_blockage ());
   else
 expand_asm_memory_blockage ();
 }

Uros.

Re: Zen tuning part 6: Break up CPU specific tuning bits to central place

[Jakub Jelinek]

On Tue, Oct 10, 2017 at 11:41:20PM +0200, Jan Hubicka wrote:
> it is common mistake to forget updating CPU spefic bits in i386.c while doing
> new CPU tuning.  It is understandble given how large i386.c is and how well
> those are hidden.  This patch moves majority of CPU specific bits into
> x86-tune-* files, organizing them according to purpose and if they are
> CPU specific.
> 
> There are no changes in code/tunings. They will come incrementally.
> 
> Bootstrapped/regtested x86_64-linux, will commit it later today.

This apparently broke bootstrap or cross build for
mingw, cygwin and x86 solaris, tested just with a cross from x86_64-linux
to i686-cygwin and i386-pc-solaris2.11.

Fixed thusly, tested on the above mentioned crosses, committed to trunk
to unbreak bootstrap.

2017-10-14  Jakub Jelinek  

PR bootstrap/82548
* config.gcc (*-*-solaris2*, i[34567]86-*-cygwin*,
x86_64-*-cygwin*, i[34567]86-*-mingw* | x86_64-*-mingw*): Append
objects to extra_objs instead of overwriting it.

--- gcc/config.gcc.jj   2017-10-11 22:37:55.0 +0200
+++ gcc/config.gcc  2017-10-14 14:20:11.294528384 +0200
@@ -874,7 +874,7 @@ case ${target} in
   tmake_file="${tmake_file} t-sol2 t-slibgcc"
   c_target_objs="${c_target_objs} sol2-c.o"
   cxx_target_objs="${cxx_target_objs} sol2-c.o sol2-cxx.o"
-  extra_objs="sol2.o sol2-stubs.o"
+  extra_objs="${extra_objs} sol2.o sol2-stubs.o"
   extra_options="${extra_options} sol2.opt"
   case ${enable_threads}:${have_pthread_h}:${have_thread_h} in
 "":yes:* | yes:yes:* )
@@ -1692,7 +1692,7 @@ i[34567]86-*-cygwin*)
tmake_file="${tmake_file} i386/t-cygming t-slibgcc"
target_gtfiles="\$(srcdir)/config/i386/winnt.c"
extra_options="${extra_options} i386/cygming.opt i386/cygwin.opt"
-   extra_objs="winnt.o winnt-stubs.o"
+   extra_objs="${extra_objs} winnt.o winnt-stubs.o"
c_target_objs="${c_target_objs} msformat-c.o"
cxx_target_objs="${cxx_target_objs} winnt-cxx.o msformat-c.o"
if test x$enable_threads = xyes; then
@@ -1708,7 +1708,7 @@ x86_64-*-cygwin*)
tmake_file="${tmake_file} i386/t-cygming t-slibgcc i386/t-cygwin-w64"
target_gtfiles="\$(srcdir)/config/i386/winnt.c"
extra_options="${extra_options} i386/cygming.opt i386/cygwin.opt"
-   extra_objs="winnt.o winnt-stubs.o"
+   extra_objs="${extra_objs} winnt.o winnt-stubs.o"
c_target_objs="${c_target_objs} msformat-c.o"
cxx_target_objs="${cxx_target_objs} winnt-cxx.o msformat-c.o"
if test x$enable_threads = xyes; then
@@ -1783,7 +1783,7 @@ i[34567]86-*-mingw* | x86_64-*-mingw*)
*)
;;
esac
-   extra_objs="winnt.o winnt-stubs.o"
+   extra_objs="${extra_objs} winnt.o winnt-stubs.o"
c_target_objs="${c_target_objs} msformat-c.o"
cxx_target_objs="${cxx_target_objs} winnt-cxx.o msformat-c.o"
gas=yes

Jakub

Re: [PATCH v2, middle-end]: Introduce memory_blockage named insn pattern

[David Edelsohn]

This patch has broken bootstrap on AIX and possibly powerpc64-linux.
Was this patch tested on any architecture other than x86?

/nasfarm/edelsohn/src/src/libgcc/emutls.c: In function '__emutls_get_address':
/nasfarm/edelsohn/src/src/libgcc/emutls.c:139:11: internal compiler
error: in invalid_void, at config/rs6000/rs6000.md:10804
   pointer offset = __atomic_load_n (&obj->loc.offset, __ATOMIC_ACQUIRE);
   ^~

Thanks, David

Re: [PATCH v2, middle-end]: Introduce memory_blockage named insn pattern

[David Edelsohn]

On Sat, Oct 14, 2017 at 11:44 AM, David Edelsohn  wrote:
> This patch has broken bootstrap on AIX and possibly powerpc64-linux.
> Was this patch tested on any architecture other than x86?
>
> /nasfarm/edelsohn/src/src/libgcc/emutls.c: In function '__emutls_get_address':
> /nasfarm/edelsohn/src/src/libgcc/emutls.c:139:11: internal compiler
> error: in invalid_void, at config/rs6000/rs6000.md:10804
>pointer offset = __atomic_load_n (&obj->loc.offset, __ATOMIC_ACQUIRE);
>^~

Same failure on powerpc64-linux using the GNU Compile Farm

during RTL pass: expand
/home/dje/src/src/libgcc/emutls.c: In function ‘__emutls_get_address’:
/home/dje/src/src/libgcc/emutls.c:139:11: internal compiler error: in
invalid_void, at config/rs6000/rs6000.md:10804
   pointer offset = __atomic_load_n (&obj->loc.offset, __ATOMIC_ACQUIRE);
   ^~

0x11401a13 invalid_void
/home/dje/src/src/gcc/config/rs6000/rs6000.md:10804
0x10c22263 expand_memory_blockage
/home/dje/src/src/gcc/optabs.c:6301
0x10c225cb expand_atomic_load(rtx_def*, rtx_def*, memmodel)
/home/dje/src/src/gcc/optabs.c:6365
0x1056a14b expand_builtin_atomic_load
/home/dje/src/src/gcc/builtins.c:5951
0x1056fd87 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
/home/dje/src/src/gcc/builtins.c:7280
0x107ecf9f expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
/home/dje/src/src/gcc/expr.c:10866
0x107de5c7 expand_expr_real(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
/home/dje/src/src/gcc/expr.c:8084
0x107d2d9b store_expr_with_bounds(tree_node*, rtx_def*, int, bool,
bool, tree_node*)
/home/dje/src/src/gcc/expr.c:5554
0x107d14e3 expand_assignment(tree_node*, tree_node*, bool)
/home/dje/src/src/gcc/expr.c:5319
0x105bb9c3 expand_call_stmt
/home/dje/src/src/gcc/cfgexpand.c:2664
0x105bf95b expand_gimple_stmt_1
/home/dje/src/src/gcc/cfgexpand.c:3585
0x105c025b expand_gimple_stmt
/home/dje/src/src/gcc/cfgexpand.c:3751
0x105c9dbf expand_gimple_basic_block
/home/dje/src/src/gcc/cfgexpand.c:5754
0x105cbf73 execute
/home/dje/src/src/gcc/cfgexpand.c:6361
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.
make: *** [emutls.o] Error 1

[Ada] Missing warning about replacement of warnings off for unreferenced

[Pierre-Marie de Rodat]

This patch corrects an issue introduced by Q220-025 where the use of pragma
warnings off applied to an unreferenced variable is not warned about the
possibility of replacing with the more specific pragma unreferenced when using
the -gnatw.w.


-- Source --


--  p.adb

procedure P is
   X : Integer;
   pragma Warnings (Off, X);
begin
   X := 12 + 53;
end;


-- Compilation and output --


& gnatmake p.adb -gnatw.w -q
p.adb:3:11: warning: could use Unreferenced instead of Warnings Off for "X"

Tested on x86_64-pc-linux-gnu, committed on trunk

2017-10-14  Justin Squirek  

* sem_elab.adb (Is_Suitable_Variable_Assignment): Replace call to
Has_Warnings_Off with Warnings_Off.

Index: sem_elab.adb
===
--- sem_elab.adb(revision 253753)
+++ sem_elab.adb(working copy)
@@ -5186,7 +5186,7 @@
 --  The variable must be a source entity and susceptible to warnings
 
 Comes_From_Source (Var_Id)
-  and then not Has_Warnings_Off (Var_Id)
+  and then not Warnings_Off (Var_Id)
 
   --  The variable must be declared in the spec of compilation unit U
 

[Ada] Proper resolution of Initializes and Initial_Condition

[Pierre-Marie de Rodat]

This patch modifies the processing of SPARK annotations Initializes and
Initial_Condition to perform the resolution of the related expressions
at the end of the enclosing package visible declarations.


-- Source --


--  init_cond.ads

package Init_Cond
  with SPARK_Mode,
   Initial_Condition =>
 Vis_Var --  OK
   and Vis_Func  --  OK
   and Vis_Nested.Var--  OK
   and Vis_Nested.Func   --  OK
   and Priv_Var  --  Error
   and Priv_Func --  Error
   and Priv_Nested.Var   --  Error
   and Priv_Nested.Func  --  Error

is
   Vis_Var : Boolean := False;
   function Vis_Func return Boolean;

   package Vis_Nested is
  Var : Boolean := True;
  function Func return Boolean;
   end Vis_Nested;

private
   Priv_Var : Boolean := False;
   function Priv_Func return Boolean;

   package Priv_Nested is
  Var : Boolean := True;
  function Func return Boolean;
   end Priv_Nested;
end Init_Cond;


-- Compilation and output --


$ gcc -c init_cond.ads
init_cond.ads:8:16: "Priv_Var" is undefined
init_cond.ads:9:16: "Priv_Func" is undefined
init_cond.ads:10:16: "Priv_Nested" is undefined (more references follow)

Tested on x86_64-pc-linux-gnu, committed on trunk

2017-10-14  Hristian Kirtchev  

* sem_ch3.adb (Analyze_Declarations): Analyze the contract of an
enclosing package at the end of the visible declarations.
* sem_prag.adb (Analyze_Initialization_Item): Suppress the analysis of
an initialization item which is undefined due to some illegality.

Index: sem_ch3.adb
===
--- sem_ch3.adb (revision 253753)
+++ sem_ch3.adb (working copy)
@@ -2820,25 +2820,11 @@
 
  --  Analyze the contracts of packages and their bodies
 
- if Nkind (Context) = N_Package_Specification then
+ if Nkind (Context) = N_Package_Specification
+   and then L = Visible_Declarations (Context)
+ then
+Analyze_Package_Contract (Defining_Entity (Context));
 
---  When a package has private declarations, its contract must be
---  analyzed at the end of the said declarations. This way both the
---  analysis and freeze actions are properly synchronized in case
---  of private type use within the contract.
-
-if L = Private_Declarations (Context) then
-   Analyze_Package_Contract (Defining_Entity (Context));
-
---  Otherwise the contract is analyzed at the end of the visible
---  declarations.
-
-elsif L = Visible_Declarations (Context)
-  and then No (Private_Declarations (Context))
-then
-   Analyze_Package_Contract (Defining_Entity (Context));
-end if;
-
  elsif Nkind (Context) = N_Package_Body then
 Analyze_Package_Body_Contract (Defining_Entity (Context));
  end if;
Index: sem_prag.adb
===
--- sem_prag.adb(revision 253753)
+++ sem_prag.adb(working copy)
@@ -2818,10 +2818,16 @@
  E_Constant,
  E_Variable)
then
+  --  When the initialization item is undefined, it appears as
+  --  Any_Id. Do not continue with the analysis of the item.
+
+  if Item_Id = Any_Id then
+ null;
+
   --  The state or variable must be declared in the visible
   --  declarations of the package (SPARK RM 7.1.5(7)).
 
-  if not Contains (States_And_Objs, Item_Id) then
+  elsif not Contains (States_And_Objs, Item_Id) then
  Error_Msg_Name_1 := Chars (Pack_Id);
  SPARK_Msg_NE
("initialization item & must appear in the visible "

[Ada] Remove obsolete comment for Generic_Parent

[Pierre-Marie de Rodat]

Routine [Set_]Generic_Parent can only be called on package, function
and procedure specification nodes, as asserted in their bodies. It would
crash when called for renaming or object declarations; the comment was
most likely referring to some earlier implemenation idea.

Tested on x86_64-pc-linux-gnu, committed on trunk

2017-10-14  Piotr Trojanek  

* sinfo.ads (Generic_Parent): Remove wrong (possibly obsolete) comment.

Index: sinfo.ads
===
--- sinfo.ads   (revision 253753)
+++ sinfo.ads   (working copy)
@@ -1472,10 +1472,7 @@
--  Generic_Parent (Node5-Sem)
--Generic_Parent is defined on declaration nodes that are instances. The
--value of Generic_Parent is the generic entity from which the instance
-   --is obtained. Generic_Parent is also defined for the renaming
-   --declarations and object declarations created for the actuals in an
-   --instantiation. The generic parent of such a declaration is the
-   --corresponding generic association in the Instantiation node.
+   --is obtained.
 
--  Generic_Parent_Type (Node4-Sem)
--Generic_Parent_Type is defined on Subtype_Declaration nodes for the

Re: [PATCH][GRAPHITE] Consistently use region analysis

[Sebastian Pop]

On Fri, Oct 13, 2017 at 8:02 AM, Richard Biener  wrote:

>
> Now that SCEV instantiation handles regions properly (see hunk below
> for a minor fix) we can use it consistently from GRAPHITE and thus
> simplify scalar_evolution_in_region greatly.
>
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
>
> A lot of the parameter renaming stuff looks dead but a more "complete"
> patch causes some more SPEC miscompares and also a bootstrap issue
> (warning only, but an uninitialized use of 'int tem = 0;' ...).
>
> This is probably all latent issues coming up more easily now.
>
> Note that formerly we'd support invariant "parameters" defined in
> the region by copying those out but now SCEV instantiation should
> lead chrec_dont_know for stuff we cannot gobble up (anythin not
> affine).  This probably only worked for the outermost scop in the
> region and it means we need some other way to handle those.


How important is it to move defs out the region?
Can we postpone handling those cases until we have an interesting case?

The
> original issue is probably that "parameters" cannot occur in
> dependences and thus an array index cannot "depend" on the computation
> of a parameter (and array indexes coming from "data" cannot be handled
> anyway?).


Correct.  Parameters can occur in array indexes as long as they cancel out.
For example, the following dependence can be computed:

A[p] vs. A[p+3]

and the following dependence cannot be computed

A[p] vs. A[0]

as the value of the parameter p is not known at compilation time.

We don't seem to have any functional testcase for those
> parameters that are not parameters.
>
>
Ok.  Let's wait for a testcase that needs this functionality.


> Richard.
>
> 2017-10-13  Richard Biener  
>
> * graphite-scop-detection.c
> (scop_detection::stmt_has_simple_data_refs_p): Always use
> the full nest as region.
> (try_generate_gimple_bb): Likewise.
> (build_scops): First split edges, then compute RPO order.
> * sese.c (scalar_evolution_in_region): Simplify now that
> SCEV can handle instantiation in regions.
> * tree-scalar-evolution.c (instantiate_scev_name): Also instantiate
> in the non-loop part of a function if requested.
>

Looks good.
Thanks.


>
> Index: gcc/graphite-scop-detection.c
> ===
> --- gcc/graphite-scop-detection.c   (revision 253721)
> +++ gcc/graphite-scop-detection.c   (working copy)
> @@ -1005,15 +1005,10 @@ scop_detection::graphite_can_represent_e
>  bool
>  scop_detection::stmt_has_simple_data_refs_p (sese_l scop, gimple *stmt)
>  {
> -  edge nest;
> +  edge nest = scop.entry;;
>loop_p loop = loop_containing_stmt (stmt);
>if (!loop_in_sese_p (loop, scop))
> -{
> -  nest = scop.entry;
> -  loop = NULL;
> -}
> -  else
> -nest = loop_preheader_edge (outermost_loop_in_sese (scop, gimple_bb
> (stmt)));
> +loop = NULL;
>
>auto_vec drs;
>if (! graphite_find_data_references_in_stmt (nest, loop, stmt, &drs))
> @@ -1381,15 +1350,10 @@ try_generate_gimple_bb (scop_p scop, bas
>vec reads = vNULL;
>
>sese_l region = scop->scop_info->region;
> -  edge nest;
> +  edge nest = region.entry;
>loop_p loop = bb->loop_father;
>if (!loop_in_sese_p (loop, region))
> -{
> -  nest = region.entry;
> -  loop = NULL;
> -}
> -  else
> -nest = loop_preheader_edge (outermost_loop_in_sese (region, bb));
> +loop = NULL;
>
>for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
> gsi_next (&gsi))
> @@ -1696,6 +1660,13 @@ build_scops (vec *scops)
>/* Now create scops from the lightweight SESEs.  */
>vec scops_l = sb.get_scops ();
>
> +  /* For our out-of-SSA we need a block on s->entry, similar to how
> + we include the LCSSA block in the region.  */
> +  int i;
> +  sese_l *s;
> +  FOR_EACH_VEC_ELT (scops_l, i, s)
> +s->entry = single_pred_edge (split_edge (s->entry));
> +
>/* Domwalk needs a bb to RPO mapping.  Compute it once here.  */
>int *postorder = XNEWVEC (int, n_basic_blocks_for_fn (cfun));
>int postorder_num = pre_and_rev_post_order_compute (NULL, postorder,
> true);
> @@ -1704,14 +1675,8 @@ build_scops (vec *scops)
>  bb_to_rpo[postorder[i]] = i;
>free (postorder);
>
> -  int i;
> -  sese_l *s;
>FOR_EACH_VEC_ELT (scops_l, i, s)
>  {
> -  /* For our out-of-SSA we need a block on s->entry, similar to how
> - we include the LCSSA block in the region.  */
> -  s->entry = single_pred_edge (split_edge (s->entry));
> -
>scop_p scop = new_scop (s->entry, s->exit);
>
>/* Record all basic blocks and their conditions in REGION.  */
> Index: gcc/sese.c
> ===
> --- gcc/sese.c  (revision 253721)
> +++ gcc/sese.c  (working copy)
> @@ -459,41 +447,16 @@ scev_analyzable_p (tree def, sese_l ®
>  tree
>  scala

[Ada] Variable assignments and reads in SPARK elaboration code

[Pierre-Marie de Rodat]

This patch reimplements the treatment of variable assignments and reads within
SPARK elaboration code. The changes are as follows:

1) Diagnostics of variable assignments in elaboration code are now based on the
   rules in effect (either Ada or SPARK).

2) Variable assignments in Ada elaboration code are considered problematic
   when a variable declared at the library level of a package spec without
   pragma Elaborate_Body lacks initialization, and the elaboration code of
   the corresponding package body initializes it. The compiler continues to
   emit a warning suggesting pragma Elaborate_Body on the package spec.

3) Variable assignments in SPARK elaboration code are considered problematic
   when a variable declared at the library level of a package spec without
   pragma Elaborate_Body is initialized, and the elaboration code of the
   corresponding package body further modifies the variable. The compiler
   emits an error on the missing Elaborate_Body.

4) A read of an external variable now imposes an Elaborate requirement on the
   unit performing the read, unless the variable is initialied, or the spec of
   the external unit carries pragma Elaborate_Body.


-- Source --


--  c1_pack.ads

with S1_Pack; use S1_Pack;

package C1_Pack with SPARK_Mode is
   Local : constant Integer := Var;--  needs Elaborate

   function Reference_Var return Boolean;
end C1_Pack;

--  c1_pack.adb

package body C1_Pack with SPARK_Mode is
   function Reference_Var return Boolean is
  procedure Read (Formal : Integer) is
  begin
 null;
  end Read;

  procedure Read_Write (Formal : in out Integer) is
  begin
 Formal := Formal + 1;
  end Read_Write;

  procedure Write (Formal : out Integer) is
  begin
 Formal := 123;
  end Write;

  Local : Integer;

   begin
  Read (Var);  --  needs Elaborate
  Read_Write (Var);--  needs Elaborate
  Local := Var;--  needs Elaborate

  Write (Var); --  OK
  Var := 234;  --  OK

  return True;
   end Reference_Var;

   Ref : constant Boolean := Reference_Var;
end C1_Pack;

--  c2_pack.ads

with S2_Pack; use S2_Pack;

package C2_Pack with SPARK_Mode is
   Local : constant Integer := Var;--  OK

   function Reference_Var return Boolean;
end C2_Pack;

--  c2_pack.adb

package body C2_Pack with SPARK_Mode is
   function Reference_Var return Boolean is
  procedure Read (Formal : Integer) is
  begin
 null;
  end Read;

  procedure Read_Write (Formal : in out Integer) is
  begin
 Formal := Formal + 1;
  end Read_Write;

  procedure Write (Formal : out Integer) is
  begin
 Formal := 123;
  end Write;

  Local : Integer;

   begin
  Read (Var);  --  OK
  Read_Write (Var);--  OK
  Local := Var;--  OK

  Write (Var); --  OK
  Var := 234;  --  OK

  return True;
   end Reference_Var;

   Ref : constant Boolean := Reference_Var;
end C2_Pack;

--  c3_pack.ads

with S3_Pack; use S3_Pack;
pragma Elaborate (S3_Pack);

package C3_Pack with SPARK_Mode is
   Local : constant Integer := Var;--  OK

   function Reference_Var return Boolean;
end C3_Pack;

--  c3_pack.adb

package body C3_Pack with SPARK_Mode is
   function Reference_Var return Boolean is
  procedure Read (Formal : Integer) is
  begin
 null;
  end Read;

  procedure Read_Write (Formal : in out Integer) is
  begin
 Formal := Formal + 1;
  end Read_Write;

  procedure Write (Formal : out Integer) is
  begin
 Formal := 123;
  end Write;

  Local : Integer;

   begin
  Read (Var);  --  OK
  Read_Write (Var);--  OK
  Local := Var;--  OK

  Write (Var); --  OK
  Var := 234;  --  OK

  return True;
   end Reference_Var;

   Ref : constant Boolean := Reference_Var;
end C3_Pack;

--  c4_pack.ads

with S4_Pack; use S4_Pack;
pragma Elaborate (S4_Pack);

package C4_Pack with SPARK_Mode is
   Local : constant Integer := Var;--  OK

   function Reference_Var return Boolean;
end C4_Pack;

--  c4_pack.adb

package body C4_Pack with SPARK_Mode is
   function Reference_Var return Boolean is
  procedure Read (Formal : Integer) is
  begin
 null;
  end Read;

  procedure Read_Write (Form

[Ada] Spurious ineffective use_type_clause warning on private type

[Pierre-Marie de Rodat]

This patch corrects an issue whereby a defaulted formal subprogram was not
being accounted for when checking for ineffective use_type_clauses on private
types used as generic actuals.


-- Source --


--  types.ads

package Types is
   type Enum_1 is private;
private
   type Enum_1 is (Red_1, Green_1, Blue_1);
end;

--  main.adb

with Types;
procedure Main is

   generic
  type Elem is private;
  with function "=" (L, R : Elem)
 return Boolean is <>;
   package Nested_4 is end;

   use type Types.Enum_1;
   package X is new Nested_4 (Types.Enum_1);
begin
   null;
end;


-- Compilation and output --


& gnatmake -q -gnatwu main.adb
main.adb:6:21: warning: function "=" is not referenced
main.adb:11:12: warning: package "X" is not referenced

Tested on x86_64-pc-linux-gnu, committed on trunk

2017-10-14  Justin Squirek  

* sem_ch8.adb (Analyze_Subprogram_Renaming): Modify condition that
triggers marking on formal subprograms.

Index: sem_ch8.adb
===
--- sem_ch8.adb (revision 253753)
+++ sem_ch8.adb (working copy)
@@ -3644,19 +3644,16 @@
   --  and mark any use_package_clauses that affect the visibility of the
   --  implicit generic actual.
 
-  if From_Default (N)
-and then Is_Generic_Actual_Subprogram (New_S)
-and then Present (Alias (New_S))
+  if Is_Generic_Actual_Subprogram (New_S)
+and then (Is_Intrinsic_Subprogram (New_S) or else From_Default (N))
   then
- Mark_Use_Clauses (Alias (New_S));
+ Mark_Use_Clauses (New_S);
 
-  --  Check intrinsic operators used as generic actuals since they may
-  --  make a use_type_clause effective.
+ --  Handle overloaded subprograms
 
-  elsif Is_Generic_Actual_Subprogram (New_S)
-and then Is_Intrinsic_Subprogram (New_S)
-  then
- Mark_Use_Clauses (New_S);
+ if Present (Alias (New_S)) then
+Mark_Use_Clauses (Alias (New_S));
+ end if;
   end if;
end Analyze_Subprogram_Renaming;
 

[Ada] Missing validity check on record type component

[Pierre-Marie de Rodat]

The compiler may silently skip generating a validity check on a
type conversion of a component of a record type. After this patch
the error is reported on the following sources.

pragma Initialize_Scalars;
package Pkg is
   type T is record
  Major : Natural;
  Minor : Natural;
   end record;

   procedure Do_Test (Value : in out T);
end;

pragma Initialize_Scalars;
package body Pkg is
   typeInteger_T is range -2 ** 31 .. 2 ** 31 - 1;
   subtype Natural_T is Integer_T range 0 .. Integer_T'Last;
   Next_Val : Integer_T := 0;

   procedure Do_Update (Int : in out Integer_T) is
   begin
  Next_Val := Next_Val + 1;
  if Next_Val > 1000 then
 Next_Val := Int;
  else
 Int := Next_Val;
  end if;
   end;

   procedure Do_Test (Value : in out T) is
   begin
  Do_Update (Natural_T (Value.Minor));   -- Run-time error
   end;

end;

with Pkg; use Pkg;
procedure Main is
   Obj : T;
begin
   Do_Test (Obj);
end Main;

Command: gnatmake -q -gnatVaM main.adb; ./main
Output:
  raised CONSTRAINT_ERROR : pkg.adb:20 invalid data

Tested on x86_64-pc-linux-gnu, committed on trunk

2017-10-14  Javier Miranda  

* checks.adb (Ensure_Valid): Do not skip adding the validity check on
renamings of objects that come from the sources.

Index: checks.adb
===
--- checks.adb  (revision 253753)
+++ checks.adb  (working copy)
@@ -5940,6 +5940,10 @@
   --  In addition, we force a check if Force_Validity_Checks is set
 
   elsif not Comes_From_Source (Expr)
+and then not
+  (Nkind (Expr) = N_Identifier
+and then Present (Renamed_Object (Entity (Expr)))
+and then Comes_From_Source (Renamed_Object (Entity (Expr
 and then not Force_Validity_Checks
 and then (Nkind (Expr) /= N_Unchecked_Type_Conversion
 or else Kill_Range_Check (Expr))

[Ada] Fix performance regression of Ada.Numerics on 32-bit Windows

[Pierre-Marie de Rodat]

This fixes a run-time performance regression recently introduced on 32-bit
Windows for Ada.Numerics by an unrelated change that exposed an old defect of
the compiler on 32-bit Windows, namely that the Long_Long_Float type has got a
wrong alignment of 8 instead of the expected 4.

The following package:

package P is
   LLF : Long_Long_Float;
end P;

must yield the following output when compiled with -gnatR2 on 32-bit Windows:

Representation information for unit P (spec)


for Llf'Size use 96;
for Llf'Alignment use 4;

Tested on x86_64-pc-linux-gnu, committed on trunk

2017-10-14  Eric Botcazou  

* cstand.adb (Build_Float_Type): Move down Siz parameter, add Align
parameter and set the alignment of the type to Align.
(Copy_Float_Type): Adjust call to Build_Float_Type.
(Register_Float_Type): Add pragma Unreferenced for Precision.  Adjust
call to Build_Float_Type and do not set RM_Size and Alignment.

Index: cstand.adb
===
--- cstand.adb  (revision 253753)
+++ cstand.adb  (working copy)
@@ -62,15 +62,22 @@
---
 
procedure Build_Float_Type
- (E: Entity_Id;
-  Siz  : Int;
-  Rep  : Float_Rep_Kind;
-  Digs : Int);
+ (E : Entity_Id;
+  Digs  : Int;
+  Rep   : Float_Rep_Kind;
+  Siz   : Int;
+  Align : Int);
--  Procedure to build standard predefined float base type. The first
-   --  parameter is the entity for the type, and the second parameter is the
-   --  size in bits. The third parameter indicates the kind of representation
-   --  to be used. The fourth parameter is the digits value. Each type
+   --  parameter is the entity for the type. The second parameter is the
+   --  digits value. The third parameter indicates the representation to
+   --  be used for the type. The fourth parameter is the size in bits.
+   --  The fifth parameter is the alignment in storage units. Each type
--  is added to the list of predefined floating point types.
+   --
+   --  Note that both RM_Size and Esize are set to the specified size, i.e.
+   --  we do not set the RM_Size to the precision passed by the back end.
+   --  This is consistent with the semantics of 'Size specified in the RM
+   --  because we cannot pack components of the type tighter than this size.
 
procedure Build_Signed_Integer_Type (E : Entity_Id; Siz : Nat);
--  Procedure to build standard predefined signed integer subtype. The
@@ -189,10 +196,11 @@
--
 
procedure Build_Float_Type
- (E: Entity_Id;
-  Siz  : Int;
-  Rep  : Float_Rep_Kind;
-  Digs : Int)
+ (E : Entity_Id;
+  Digs  : Int;
+  Rep   : Float_Rep_Kind;
+  Siz   : Int;
+  Align : Int)
is
begin
   Set_Type_Definition (Parent (E),
@@ -201,10 +209,10 @@
 
   Set_Ekind  (E, E_Floating_Point_Type);
   Set_Etype  (E, E);
-  Set_Float_Rep (E, Rep);
+  Init_Digits_Value  (E, Digs);
+  Set_Float_Rep  (E, Rep);
   Init_Size  (E, Siz);
-  Set_Elem_Alignment (E);
-  Init_Digits_Value  (E, Digs);
+  Set_Alignment  (E, UI_From_Int (Align));
   Set_Float_Bounds   (E);
   Set_Is_Frozen  (E);
   Set_Is_Public  (E);
@@ -295,8 +303,9 @@
 
procedure Copy_Float_Type (To : Entity_Id; From : Entity_Id) is
begin
-  Build_Float_Type (To, UI_To_Int (Esize (From)), Float_Rep (From),
-UI_To_Int (Digits_Value (From)));
+  Build_Float_Type
+(To, UI_To_Int (Digits_Value (From)), Float_Rep (From),
+ UI_To_Int (Esize (From)), UI_To_Int (Alignment (From)));
end Copy_Float_Type;
 
--
@@ -2065,15 +2074,17 @@
   Size  : Positive;
   Alignment : Natural)
is
+  pragma Unreferenced (Precision);
+  --  See Build_Float_Type for the rationale
+
   Ent : constant Entity_Id := New_Standard_Entity;
 
begin
   Set_Defining_Identifier (New_Node (N_Full_Type_Declaration, Stloc), Ent);
   Make_Name (Ent, Name);
   Set_Scope (Ent, Standard_Standard);
-  Build_Float_Type (Ent, Int (Size), Float_Rep, Pos (Digs));
-  Set_RM_Size (Ent, UI_From_Int (Int (Precision)));
-  Set_Alignment (Ent, UI_From_Int (Int (Alignment / 8)));
+  Build_Float_Type
+(Ent, Pos (Digs), Float_Rep, Int (Size), Int (Alignment / 8));
 
   if No (Back_End_Float_Types) then
  Back_End_Float_Types := New_Elmt_List;

[Ada] Premature evaluation of message string in Assert pragma

[Pierre-Marie de Rodat]

RM 11.4.2 stipulates that the optional string argument in an Assert pragma is
evaluated only if the assertion fails and the string is incorporated into the
raise statement. Previous to this patch the string expression was evaluated
unconditionally, leading to unwanted side effects if its evaluation only
made sense in case of failure of the assertion.

Executing:

   gnatmake -gnata -gnatws -q main
   main

must yield:

   Assert succeeds

   raised SYSTEM.ASSERTIONS.ASSERT_FAILURE : P should be null, got A_STRING

---
with Text_IO; use Text_IO;
procedure Main is
  P : access String;
  X : Integer;
  function Zero return Integer is begin return 0; end;

begin
  X := Zero;
  pragma Assert (P = null, "P should be null, got " & P.all);
  Put_Line ("Assert succeeds");

  if X = 0 then
  P := new String'("A_STRING");
   end if;

  pragma Assert (P = null, "P should be null, got " & P.all);
end Main;

Tested on x86_64-pc-linux-gnu, committed on trunk

2017-10-14  Ed Schonberg  

* sem_prag.adb (Analyze_Pragma, case Check): Defer evaluation of the
optional string in an Assert pragma until the expansion of the pragma
has rewritten it as a conditional statement, so that the string
argument is only evaluaed if the assertion fails. This is mandated by
RM 11.4.2.

Index: sem_prag.adb
===
--- sem_prag.adb(revision 253754)
+++ sem_prag.adb(working copy)
@@ -13249,16 +13249,18 @@
--  If checks are not on we don't want any expansion (since
--  such expansion would not get properly deleted) but
--  we do want to analyze (to get proper references).
-   --  The Preanalyze_And_Resolve routine does just what we want
+   --  The Preanalyze_And_Resolve routine does just what we want.
+   --  Ditto if pragma is active, because it will be rewritten
+   --  as an if-statement whose analysis will complete analysis
+   --  and expansion of the string message. This makes a
+   --  difference in the unusual case where the expression for
+   --  the string may have a side effect, such as raising an
+   --  exception. This is mandated by RM 11.4.2, which specifies
+   --  that the string expression is only evaluated if the
+   --  check fails and Assertion_Error is to be raised.
 
-   if Is_Ignored (N) then
-  Preanalyze_And_Resolve (Str, Standard_String);
+   Preanalyze_And_Resolve (Str, Standard_String);
 
-  --  Otherwise we need a proper analysis and expansion
-
-   else
-  Analyze_And_Resolve (Str, Standard_String);
-   end if;
 end if;
 
 --  Now you might think we could just do the same with the Boolean

[Ada] Repair ABI breakage on 32-bit x86/Linux

[Pierre-Marie de Rodat]

This repairs the ABI breakage for record types with Long_Float components
introduced on 32-bit x86/Linux by the previous change.  The Long_Float type
is awkward on this platform because it has got a dual alignment setting:
it's 8 for standalone object and array component and 4 for record component.
Since Ada defines a single 'Alignment value, it is set to 4 and there is a
special circuitry in Set_Elem_Alignment to implement it.

The previous change short-circuited Set_Elem_Alignment in Build_Float_Type,
which resulted in a Long_Float'Alignment value of 8.

The following package:

package P is

  type Rec is record
I : Integer;
F : Long_Float;
  end record;

end P;

must yield the following output when compiled with -gnatR2 on 32-bit Linux:

Representation information for unit P (spec)

for Rec'Size use 96;
for Rec'Alignment use 4;
for Rec use record
   I at 0 range  0 .. 31;
   F at 4 range  0 .. 63;
end record;

Tested on x86_64-pc-linux-gnu, committed on trunk

2017-10-14  Eric Botcazou  

* layout.ads (Set_Elem_Alignment): Add Align parameter defaulted to 0.
* layout.adb (Set_Elem_Alignment): Likewise.  Use M name as maximum
alignment for consistency.  If Align is non-zero, use the minimum of
Align and M for the alignment.
* cstand.adb (Build_Float_Type): Use Set_Elem_Alignment instead of
setting the alignment directly.

Index: cstand.adb
===
--- cstand.adb  (revision 253756)
+++ cstand.adb  (working copy)
@@ -212,7 +212,7 @@
   Init_Digits_Value  (E, Digs);
   Set_Float_Rep  (E, Rep);
   Init_Size  (E, Siz);
-  Set_Alignment  (E, UI_From_Int (Align));
+  Set_Elem_Alignment (E, Align);
   Set_Float_Bounds   (E);
   Set_Is_Frozen  (E);
   Set_Is_Public  (E);
Index: layout.adb
===
--- layout.adb  (revision 253753)
+++ layout.adb  (working copy)
@@ -843,7 +843,7 @@
-- Set_Elem_Alignment --

 
-   procedure Set_Elem_Alignment (E : Entity_Id) is
+   procedure Set_Elem_Alignment (E : Entity_Id; Align : Nat := 0) is
begin
   --  Do not set alignment for packed array types, this is handled in the
   --  backend.
@@ -869,16 +869,13 @@
  return;
   end if;
 
-  --  Here we calculate the alignment as the largest power of two multiple
-  --  of System.Storage_Unit that does not exceed either the object size of
-  --  the type, or the maximum allowed alignment.
+  --  We attempt to set the alignment in all the other cases
 
   declare
  S : Int;
  A : Nat;
+ M : Nat;
 
- Max_Alignment : Nat;
-
   begin
  --  The given Esize may be larger that int'last because of a previous
  --  error, and the call to UI_To_Int will fail, so use default.
@@ -908,7 +905,7 @@
and then S = 8
and then Is_Floating_Point_Type (E)
  then
-Max_Alignment := Ttypes.Target_Double_Float_Alignment;
+M := Ttypes.Target_Double_Float_Alignment;
 
  --  If the default alignment of "double" or larger scalar types is
  --  specifically capped, enforce the cap.
@@ -917,19 +914,28 @@
and then S >= 8
and then Is_Scalar_Type (E)
  then
-Max_Alignment := Ttypes.Target_Double_Scalar_Alignment;
+M := Ttypes.Target_Double_Scalar_Alignment;
 
  --  Otherwise enforce the overall alignment cap
 
  else
-Max_Alignment := Ttypes.Maximum_Alignment;
+M := Ttypes.Maximum_Alignment;
  end if;
 
- A := 1;
- while 2 * A <= Max_Alignment and then 2 * A <= S loop
-A := 2 * A;
- end loop;
+ --  We calculate the alignment as the largest power-of-two multiple
+ --  of System.Storage_Unit that does not exceed the object size of
+ --  the type and the maximum allowed alignment, if none was specified.
+ --  Otherwise we only cap it to the maximum allowed alignment.
 
+ if Align = 0 then
+A := 1;
+while 2 * A <= S and then 2 * A <= M loop
+   A := 2 * A;
+end loop;
+ else
+A := Nat'Min (Align, M);
+ end if;
+
  --  If alignment is currently not set, then we can safely set it to
  --  this new calculated value.
 
Index: layout.ads
===
--- layout.ads  (revision 253753)
+++ layout.ads  (working copy)
@@ -74,10 +74,11 @@
--  types, the RM_Size is simply set to zero. This routine also sets
--  the Is_Constrained flag in Def_Id.
 
-   procedure Set_Elem_Alignment (E : Entity_Id);
+   procedure Set_Elem_Alignment (E : Enti

[Ada] Activation/suppression of SPARK elaboration rules

[Pierre-Marie de Rodat]

This patch utilizes compilation switch -gnatd.v to enforce the SPARK rules for
elaboration in SPARK code. The affected scenarios are calls and instantiations.
If the switch is active, the ABE mechanism will verify that the scenarios have
fulfilled their Elaborate[_All] requirements. Otherwise the static model of the
ABE mechanism will install implicit Elaborate[_All] pragmas to meet these
requirements.


-- Source --


--  server.ads

package Server with SPARK_Mode is
   generic
   procedure Gen_Proc;

   generic
   package Gen_Pack is
  procedure Proc;
   end Gen_Pack;

   function Func return Boolean;
end Server;

--  server.adb

with Ada.Text_IO; use Ada.Text_IO;

package body Server with SPARK_Mode is
   procedure Gen_Proc is
   begin
  Put_Line ("Gen_Proc");
   end Gen_Proc;

   package body Gen_Pack is
  procedure Proc is
  begin
 Put_Line ("Proc");
  end Proc;
   end Gen_Pack;

   function Func return Boolean is
   begin
  Put_Line ("Func");
  return True;
   end Func;
end Server;

--  client.ads

with Server;

package Client with SPARK_Mode is
   procedure Inst_Proc is new Server.Gen_Proc;
   package   Inst_Pack is new Server.Gen_Pack;

   Val : constant Boolean := Server.Func;
end Client;


-- Compilation and output --


$ echo "Ignore SPARK rules"
$ gcc -c client.ads
$ echo "Apply SPARK rules"
$ gcc -c client.ads -gnatd.v
Ignore SPARK rules
Apply SPARK rules
client.ads:4:04: instantiation of "Gen_Proc" during elaboration in SPARK
client.ads:4:04: unit "Client" requires pragma "Elaborate_All" for "Server"
client.ads:4:04:   spec of unit "Client" elaborated
client.ads:4:04:   procedure "Gen_Proc" instantiated as "Inst_Proc" at line 4
client.ads:5:04: instantiation of "Gen_Pack" during elaboration in SPARK
client.ads:5:04: unit "Client" requires pragma "Elaborate" for "Server"
client.ads:5:04:   spec of unit "Client" elaborated
client.ads:5:04:   package "Gen_Pack" instantiated as "Inst_Pack" at line 5
client.ads:7:36: call to "Func" during elaboration in SPARK
client.ads:7:36: unit "Client" requires pragma "Elaborate_All" for "Server"
client.ads:7:36:   spec of unit "Client" elaborated
client.ads:7:36:   function "Func" called at line 7

Tested on x86_64-pc-linux-gnu, committed on trunk

2017-10-14  Hristian Kirtchev  

* debug.adb: Switch -gnatd.v and associated flag are now used to
enforce the SPARK rules for elaboration in SPARK code.
* sem_elab.adb: Describe switch -gnatd.v.
(Process_Call): Verify the SPARK rules only when -gnatd.v is in effect.
(Process_Instantiation): Verify the SPARK rules only when -gnatd.v is
in effect.
(Process_Variable_Assignment): Clarify why variable assignments are
processed reglardless of whether -gnatd.v is in effect.
* doc/gnat_ugn/elaboration_order_handling_in_gnat.rst: Update the
sections on elaboration code and compilation switches.
* gnat_ugn.texi: Regenerate.

Index: doc/gnat_ugn/elaboration_order_handling_in_gnat.rst
===
--- doc/gnat_ugn/elaboration_order_handling_in_gnat.rst (revision 253753)
+++ doc/gnat_ugn/elaboration_order_handling_in_gnat.rst (working copy)
@@ -133,9 +133,44 @@
 =
 
 The sequence by which the elaboration code of all units within a partition is
-executed is referred to as **elaboration order**. The elaboration order depends
-on the following factors:
+executed is referred to as **elaboration order**.
 
+Within a single unit, elaboration code is executed in sequential order.
+
+::
+
+   package body Client is
+  Result : ... := Server.Func;
+
+  procedure Proc is
+ package Inst is new Server.Gen;
+  begin
+ Inst.Eval (Result);
+  end Proc;
+   begin
+  Proc;
+   end Client;
+
+In the example above, the elaboration order within package body ``Client`` is
+as follows:
+
+1. The object declaration of ``Result`` is elaborated.
+
+   * Function ``Server.Func`` is invoked.
+
+2. The subprogram body of ``Proc`` is elaborated.
+
+3. Procedure ``Proc`` is invoked.
+
+   * Generic unit ``Server.Gen`` is instantiated as ``Inst``.
+
+   * Instance ``Inst`` is elaborated.
+
+   * Procedure ``Inst.Eval`` is invoked.
+
+The elaboration order of all units within a partition depends on the following
+factors:
+
 * |withed| units
 
 * purity of units
@@ -571,7 +606,7 @@
   a partition is elaboration code. GNAT performs very few diagnostics and
   generates run-time checks to verify the elaboration order of a program. This
   behavior is identical to that specified by the Ada Reference Manual. The
-  dynamic model is enabled with compilation switch :switch:`-gnatE`.
+  dynamic model is enabled with compiler switch :switch:`-gnatE`.
 
 .. index:: Static elaboration model
 
@@ -860,7 +895,7 @@
 The SPARK model is identical to the static model in

[Ada] Calls in preelaborated units and pragma Remote_Call_Interface

[Pierre-Marie de Rodat]

This patch modifies the check which ensures that no call is executed in a
preelaborated unit. The check now properly ignores a case where a generic
unit is subject to pragma Remote_Call_Interface, and the call appears in
the body.

Tested on x86_64-pc-linux-gnu, committed on trunk

gcc/ada/

2017-10-14  Hristian Kirtchev  

* sem_elab.adb (In_Preelaborated_Context): A generic package subject to
Remote_Call_Interface is not a suitable preelaboratd context when the
call appears in the package body.

gcc/testsuite/

2017-10-14  Hristian Kirtchev  

* gnat.dg/remote_call_iface.ads, gnat.dg/remote_call_iface.adb: New
testcase.
Index: sem_elab.adb
===
--- sem_elab.adb(revision 253757)
+++ sem_elab.adb(working copy)
@@ -1808,7 +1808,7 @@
  --  be on another machine.
 
  if Ekind (Body_Id) = E_Package_Body
-   and then Ekind (Spec_Id) = E_Package
+   and then Ekind_In (Spec_Id, E_Generic_Package, E_Package)
and then (Is_Remote_Call_Interface (Spec_Id)
   or else Is_Remote_Types (Spec_Id))
  then
Index: ../testsuite/gnat.dg/remote_call_iface.ads
===
--- ../testsuite/gnat.dg/remote_call_iface.ads  (revision 0)
+++ ../testsuite/gnat.dg/remote_call_iface.ads  (revision 0)
@@ -0,0 +1,5 @@
+generic
+package Remote_Call_Iface is
+   pragma Remote_Call_Interface;
+   procedure Proc;
+end Remote_Call_Iface;
Index: ../testsuite/gnat.dg/remote_call_iface.adb
===
--- ../testsuite/gnat.dg/remote_call_iface.adb  (revision 0)
+++ ../testsuite/gnat.dg/remote_call_iface.adb  (revision 0)
@@ -0,0 +1,7 @@
+--  { dg-do compile }
+
+package body Remote_Call_Iface is
+   procedure Proc is begin null; end;
+begin
+   Proc;
+end Remote_Call_Iface;

Re: [PATCH v2, middle-end]: Introduce memory_blockage named insn pattern

[Uros Bizjak]

On Sat, Oct 14, 2017 at 5:44 PM, David Edelsohn  wrote:
> This patch has broken bootstrap on AIX and possibly powerpc64-linux.
> Was this patch tested on any architecture other than x86?

No.

> /nasfarm/edelsohn/src/src/libgcc/emutls.c: In function '__emutls_get_address':
> /nasfarm/edelsohn/src/src/libgcc/emutls.c:139:11: internal compiler
> error: in invalid_void, at config/rs6000/rs6000.md:10804
>pointer offset = __atomic_load_n (&obj->loc.offset, __ATOMIC_ACQUIRE);
>^~

Basically, the patch does only:

+expand_memory_blockage (void)
+{
+  if (targetm.have_memory_blockage)
+emit_insn (targetm.gen_memory_blockage ());
+  else
+expand_asm_memory_blockage ();
+}

So, if the target doesn't declare memory_blockage pattern, as is the
case with rs6000, I really fail to see what could go wrong here.

Uros.

Re: [PATCH v2, middle-end]: Introduce memory_blockage named insn pattern

[David Edelsohn]

On Sat, Oct 14, 2017 at 1:29 PM, Uros Bizjak  wrote:
> On Sat, Oct 14, 2017 at 5:44 PM, David Edelsohn  wrote:
>> This patch has broken bootstrap on AIX and possibly powerpc64-linux.
>> Was this patch tested on any architecture other than x86?
>
> No.
>
>> /nasfarm/edelsohn/src/src/libgcc/emutls.c: In function 
>> '__emutls_get_address':
>> /nasfarm/edelsohn/src/src/libgcc/emutls.c:139:11: internal compiler
>> error: in invalid_void, at config/rs6000/rs6000.md:10804
>>pointer offset = __atomic_load_n (&obj->loc.offset, __ATOMIC_ACQUIRE);
>>^~
>
> Basically, the patch does only:
>
> +expand_memory_blockage (void)
> +{
> +  if (targetm.have_memory_blockage)
> +emit_insn (targetm.gen_memory_blockage ());
> +  else
> +expand_asm_memory_blockage ();
> +}
>
> So, if the target doesn't declare memory_blockage pattern, as is the
> case with rs6000, I really fail to see what could go wrong here.

Thanks for finding and fixing the targetm.have_memory_blockage typo.

- David

Re: [PATCH] Improve simplify_rotate (PR middle-end/62263, PR middle-end/82498)

[Richard Biener]

On October 13, 2017 9:36:48 PM GMT+02:00, Jakub Jelinek  
wrote:
>Hi!
>
>The forwprop rotate pattern recognizer is able to detect various
>patterns, but for the case where we want to support all rotate
>counts without UB, it requires
>Y &= B - 1;
>R = (X << Y) | (X >> ((-Y) & (B - 1)));
>where
>R = (X << (Y & (B - 1))) | (X >> ((-Y) & (B - 1)));
>is another reasonable way to write the same thing (don't mask it
>for the case of negation twice).
>
>The following patch teaches forwprop to handle that.
>
>Also, we weren't recognizing rotates of constant X by variable
>shift count Y.
>
>And finally, I've noticed there is a missing check that B is a power of
>two, which matters for the & (B - 1) style patterns - if it is
>not a pow2p, then it isn't doing what we expect it to be.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK. 

Richard. 

>2017-10-13  Jakub Jelinek  
>
>   PR middle-end/62263
>   PR middle-end/82498
>   * tree-ssa-forwprop.c (simplify_rotate): Allow def_arg1[N]
>   to be any operand_equal_p operands.  For & (B - 1) require
>   B to be power of 2.  Recognize
>   (X << (Y & (B - 1))) | (X >> ((-Y) & (B - 1))) and similar patterns.
>
>   * c-c++-common/rotate-5.c (f2): New function.  Move old
>   function to ...
>   (f4): ... this.  Use 127 instead of 128.
>   (f3, f5, f6): New functions.
>   (main): Test all f[1-6] functions, with both 0 and 1 as
>   second arguments.
>   * c-c++-common/rotate-6.c: New test.
>   * c-c++-common/rotate-6a.c: New test.
>   * c-c++-common/rotate-7.c: New test.
>   * c-c++-common/rotate-7a.c: New test.
>   * c-c++-common/rotate-8.c: New test.
>
>--- gcc/tree-ssa-forwprop.c.jj 2017-09-14 22:15:12.0 +0200
>+++ gcc/tree-ssa-forwprop.c2017-10-13 14:35:31.763191645 +0200
>@@ -1491,9 +1491,14 @@ defcodefor_name (tree name, enum tree_co
>applied, otherwise return false.
> 
>We are looking for X with unsigned type T with bitsize B, OP being
>-   +, | or ^, some type T2 wider than T and
>+   +, | or ^, some type T2 wider than T.  For:
>(X << CNT1) OP (X >> CNT2) iff CNT1 + CNT2 == B
>  ((T) ((T2) X << CNT1)) OP ((T) ((T2) X >> CNT2)) iff CNT1 + CNT2 == B
>+
>+   transform these into:
>+   X r<< CNT1
>+
>+   Or for:
>(X << Y) OP (X >> (B - Y))
>(X << (int) Y) OP (X >> (int) (B - Y))
>((T) ((T2) X << Y)) OP ((T) ((T2) X >> (B - Y)))
>@@ -1503,12 +1508,23 @@ defcodefor_name (tree name, enum tree_co
>((T) ((T2) X << Y)) | ((T) ((T2) X >> ((-Y) & (B - 1
>   ((T) ((T2) X << (int) Y)) | ((T) ((T2) X >> (int) ((-Y) & (B - 1
> 
>-   and transform these into:
>-   X r<< CNT1
>+   transform these into:
>X r<< Y
> 
>+   Or for:
>+   (X << (Y & (B - 1))) | (X >> ((-Y) & (B - 1)))
>+   (X << (int) (Y & (B - 1))) | (X >> (int) ((-Y) & (B - 1)))
>+   ((T) ((T2) X << (Y & (B - 1 | ((T) ((T2) X >> ((-Y) & (B -
>1
>+   ((T) ((T2) X << (int) (Y & (B - 1 \
>+ | ((T) ((T2) X >> (int) ((-Y) & (B - 1
>+
>+   transform these into:
>+   X r<< (Y & (B - 1))
>+
>Note, in the patterns with T2 type, the type of OP operands
>-   might be even a signed type, but should have precision B.  */
>+   might be even a signed type, but should have precision B.
>+   Expressions with & (B - 1) should be recognized only if B is
>+   a power of 2.  */
> 
> static bool
> simplify_rotate (gimple_stmt_iterator *gsi)
>@@ -1578,7 +1594,9 @@ simplify_rotate (gimple_stmt_iterator *g
>   def_arg1[i] = tem;
>   }
>   /* Both shifts have to use the same first operand.  */
>-  if (TREE_CODE (def_arg1[0]) != SSA_NAME || def_arg1[0] !=
>def_arg1[1])
>+  if (!operand_equal_for_phi_arg_p (def_arg1[0], def_arg1[1])
>+  || !types_compatible_p (TREE_TYPE (def_arg1[0]),
>+TREE_TYPE (def_arg1[1])))
> return false;
>   if (!TYPE_UNSIGNED (TREE_TYPE (def_arg1[0])))
> return false;
>@@ -1649,8 +1667,10 @@ simplify_rotate (gimple_stmt_iterator *g
>   /* The above sequence isn't safe for Y being 0,
>  because then one of the shifts triggers undefined behavior.
>  This alternative is safe even for rotation count of 0.
>- One shift count is Y and the other (-Y) & (B - 1).  */
>+ One shift count is Y and the other (-Y) & (B - 1).
>+ Or one shift count is Y & (B - 1) and the other (-Y) & (B - 1). 
>*/
>   else if (cdef_code[i] == BIT_AND_EXPR
>+   && pow2p_hwi (TYPE_PRECISION (rtype))
>&& tree_fits_shwi_p (cdef_arg2[i])
>&& tree_to_shwi (cdef_arg2[i])
>   == TYPE_PRECISION (rtype) - 1
>@@ -1675,17 +1695,50 @@ simplify_rotate (gimple_stmt_iterator *g
>   rotcnt = tem;
>   break;
> }
>-  defcodefor_name (tem, &code, &tem, NULL);
>+  tree tem2;
>+  defcodefor_name (tem, &code, &tem2, NULL);
>   if (CONVERT_E

Re: [PATCH] Slightly improve phiopt value_replacement optimization (PR middle-end/62263, PR middle-end/82498)

[Richard Biener]

On October 13, 2017 9:43:10 PM GMT+02:00, Jakub Jelinek  
wrote:
>Hi!
>
>First of all, there was a typo, we are optimizing
>(x != 0) ? x + y : y
>into x + y rather than y.
>
>And, as the comment mentions, the condition that there is just a single
>stmt is too restrictive and in the various patterns posted in the
>various
>rotate related PRs there are cases where in addition to the last
>assign stmt there are some preparation statements (sometimes
>conversion,
>sometimes bit masking with constant, sometimes both).
>I think it is undesirable to turn the conditional code into always
>executed
>one if it is too large, so this patch allows just very few very cheap
>preparation statements which feed each other and it is easily possible
>to
>propagate the cond_rhs constant through those statements to compute
>what
>the result from those would be (and make sure no UB).
>
>With this patch on top of the previous one, on rotate-8.c testcase
>on x86_64-linux and i686-linux we get the most efficient code in all
>cases
>- just a rol/ror instruction with perhaps some moves into registers
>needed
>to perform that, but without any conditionals, masking etc.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK. 

Richard. 

>2017-10-13  Jakub Jelinek  
>
>   PR middle-end/62263
>   PR middle-end/82498
>   * tree-ssa-phiopt.c (value_replacement): Comment fix.  Handle
>   up to 2 preparation statements for ASSIGN in MIDDLE_BB.
>
>   * c-c++-common/rotate-8.c: Expect no PHIs in optimized dump.
>
>--- gcc/tree-ssa-phiopt.c.jj   2017-10-10 22:04:08.0 +0200
>+++ gcc/tree-ssa-phiopt.c  2017-10-13 17:55:01.578450763 +0200
>@@ -995,11 +995,13 @@ value_replacement (basic_block cond_bb,
> 
> }
> 
>-  /* Now optimize (x != 0) ? x + y : y to just y.
>- The following condition is too restrictive, there can easily be
>another
>- stmt in middle_bb, for instance a CONVERT_EXPR for the second
>argument.  */
>-  gimple *assign = last_and_only_stmt (middle_bb);
>-  if (!assign || gimple_code (assign) != GIMPLE_ASSIGN
>+  /* Now optimize (x != 0) ? x + y : y to just x + y.  */
>+  gsi = gsi_last_nondebug_bb (middle_bb);
>+  if (gsi_end_p (gsi))
>+return 0;
>+
>+  gimple *assign = gsi_stmt (gsi);
>+  if (!is_gimple_assign (assign)
>   || gimple_assign_rhs_class (assign) != GIMPLE_BINARY_RHS
>   || (!INTEGRAL_TYPE_P (TREE_TYPE (arg0))
> && !POINTER_TYPE_P (TREE_TYPE (arg0
>@@ -1009,6 +1011,71 @@ value_replacement (basic_block cond_bb,
>   if (!gimple_seq_empty_p (phi_nodes (middle_bb)))
> return 0;
> 
>+  /* Allow up to 2 cheap preparation statements that prepare argument
>+ for assign, e.g.:
>+  if (y_4 != 0)
>+  goto ;
>+  else
>+  goto ;
>+ :
>+  _1 = (int) y_4;
>+  iftmp.0_6 = x_5(D) r<< _1;
>+ :
>+  # iftmp.0_2 = PHI 
>+ or:
>+  if (y_3(D) == 0)
>+  goto ;
>+  else
>+  goto ;
>+ :
>+  y_4 = y_3(D) & 31;
>+  _1 = (int) y_4;
>+  _6 = x_5(D) r<< _1;
>+ :
>+  # _2 = PHI   */
>+  gimple *prep_stmt[2] = { NULL, NULL };
>+  int prep_cnt;
>+  for (prep_cnt = 0; ; prep_cnt++)
>+{
>+  gsi_prev_nondebug (&gsi);
>+  if (gsi_end_p (gsi))
>+  break;
>+
>+  gimple *g = gsi_stmt (gsi);
>+  if (gimple_code (g) == GIMPLE_LABEL)
>+  break;
>+
>+  if (prep_cnt == 2 || !is_gimple_assign (g))
>+  return 0;
>+
>+  tree lhs = gimple_assign_lhs (g);
>+  tree rhs1 = gimple_assign_rhs1 (g);
>+  use_operand_p use_p;
>+  gimple *use_stmt;
>+  if (TREE_CODE (lhs) != SSA_NAME
>+|| TREE_CODE (rhs1) != SSA_NAME
>+|| !INTEGRAL_TYPE_P (TREE_TYPE (lhs))
>+|| !INTEGRAL_TYPE_P (TREE_TYPE (rhs1))
>+|| !single_imm_use (lhs, &use_p, &use_stmt)
>+|| use_stmt != (prep_cnt ? prep_stmt[prep_cnt - 1] : assign))
>+  return 0;
>+  switch (gimple_assign_rhs_code (g))
>+  {
>+  CASE_CONVERT:
>+break;
>+  case PLUS_EXPR:
>+  case BIT_AND_EXPR:
>+  case BIT_IOR_EXPR:
>+  case BIT_XOR_EXPR:
>+if (TREE_CODE (gimple_assign_rhs2 (g)) != INTEGER_CST)
>+  return 0;
>+break;
>+  default:
>+return 0;
>+  }
>+  prep_stmt[prep_cnt] = g;
>+}
>+
>   /* Only transform if it removes the condition.  */
>if (!single_non_singleton_phi_for_edges (phi_nodes (gimple_bb (phi)),
>e0, e1))
> return 0;
>@@ -1019,7 +1086,7 @@ value_replacement (basic_block cond_bb,
>   && profile_status_for_fn (cfun) != PROFILE_ABSENT
>&& EDGE_PRED (middle_bb, 0)->probability < profile_probability::even ()
>   /* If assign is cheap, there is no point avoiding it.  */
>-  && estimate_num_insns (assign, &eni_time_weights)
>+  && estimate_num_insns (bb_seq (middle_bb), &eni_time_weights)
>>= 3 * estimate_num_insns (cond, &eni_time_weights))
> return 0;
> 
>@@ -1030,6 +1097,32 @@ value_replacement (basic_block cond_bb,
>   tree cond_lhs = gimple_cond