Re: [patch] Couple of tweaks to the gimplifier
On Mon, Mar 21, 2011 at 6:48 PM, Eric Botcazou wrote: >> > 1) Set TREE_THIS_NOTRAP on the INDIRECT_REF built for VLA decls. This >> > is correct since stack memory isn't considered as trapping in the IL. >> >> This is ok. > > Thanks. > >> > 2) Improve gimplification of complex conditions in COND_EXPR. They are >> > naturally generated by the Ada compiler and the patch avoids emitting >> > redundant branches in GIMPLE, visible at -O0 for the testcase: >> >> Shouldn't >> >> + /* Remove any COMPOUND_EXPR so the following cases will be caught. */ >> + STRIP_TYPE_NOPS (TREE_OPERAND (expr, 0)); >> + if (TREE_CODE (TREE_OPERAND (expr, 0)) == COMPOUND_EXPR) >> + gimplify_compound_expr (&TREE_OPERAND (expr, 0), pre_p, true); >> >> happen in gimple_boolify instead so that other callers also benefit? >> That is, add a COMPOUND_EXPR case there? > > Not clear to me. gimple_boolify doesn't gimplify, it boolifies, i.e. only > does > type conversions to boolean. This looks orthogonal. > >> So, what does the GENERIC look like here? > > Attached. Barely readable, like pretty much all GENERIC for Ada, but you can > see the big IF statement with the COMPOUND_EXPR on the RHS of the &&. So looking at the GENERIC I fail to see how the patch would handle the COMPOUND_EXPR which is in operand 1 of the &&. That's also one reason I suggested gimple_boolify instead, as that works recursively on the predicate. Of course you are right, gimple_boolify doesn't seem to be prepared to do gimplification. Ok, debugging. Ah, I see - we recursively gimplify the pieces of the predicate. So yes, I think your patch makes sense. Thus, ok. Thanks, Richard. > -- > Eric Botcazou >
Re: [patch] Couple of tweaks to the gimplifier
> > 1) Set TREE_THIS_NOTRAP on the INDIRECT_REF built for VLA decls. This
> > is correct since stack memory isn't considered as trapping in the IL.
>
> This is ok.
Thanks.
> > 2) Improve gimplification of complex conditions in COND_EXPR. They are
> > naturally generated by the Ada compiler and the patch avoids emitting
> > redundant branches in GIMPLE, visible at -O0 for the testcase:
>
> Shouldn't
>
> + /* Remove any COMPOUND_EXPR so the following cases will be caught. */
> + STRIP_TYPE_NOPS (TREE_OPERAND (expr, 0));
> + if (TREE_CODE (TREE_OPERAND (expr, 0)) == COMPOUND_EXPR)
> +gimplify_compound_expr (&TREE_OPERAND (expr, 0), pre_p, true);
>
> happen in gimple_boolify instead so that other callers also benefit?
> That is, add a COMPOUND_EXPR case there?
Not clear to me. gimple_boolify doesn't gimplify, it boolifies, i.e. only does
type conversions to boolean. This looks orthogonal.
> So, what does the GENERIC look like here?
Attached. Barely readable, like pretty much all GENERIC for Ada, but you can
see the big IF statement with the COMPOUND_EXPR on the RHS of the &&.
--
Eric Botcazou
P (const boolean b, struct s1, struct s2)
{
SAVE_EXPR LB0>;
SAVE_EXPR UB0>;
SAVE_EXPR UB0> >= SAVE_EXPR LB0> ? (SAVE_EXPR UB0> - SAVE_EXPR LB0>) + 1 : 0>;
SAVE_EXPR LB0>;
SAVE_EXPR UB0>;
SAVE_EXPR UB0> >= SAVE_EXPR LB0> ? (SAVE_EXPR UB0> - SAVE_EXPR LB0>) + 1 : 0>;
{
typedef p__TS2bP1___XD p__TS2bP1___XD;
typedef struct ;
typedef character p__S2b[(size_type) SAVE_EXPR LB0>:SAVE_EXPR UB0> >= SAVE_EXPR LB0> ? (size_type) SAVE_EXPR UB0> : (size_type) SAVE_EXPR LB0> + -1];
typedef p__TS1bP1___XD p__TS1bP1___XD;
typedef struct ;
typedef character p__S1b[(size_type) SAVE_EXPR LB0>:SAVE_EXPR UB0> >= SAVE_EXPR LB0> ? (size_type) SAVE_EXPR UB0> : (size_type) SAVE_EXPR LB0> + -1];
const integer L3b = (const integer) (SAVE_EXPR UB0> >= SAVE_EXPR LB0> ? (SAVE_EXPR UB0> - SAVE_EXPR LB0>) + 1 : 0>);
const integer L4b = (const integer) (SAVE_EXPR UB0> >= SAVE_EXPR LB0> ? (SAVE_EXPR UB0> - SAVE_EXPR LB0>) + 1 : 0>);
const integer L5b = (const integer) ((integer) L3b + (integer) L4b);
const integer L6b = (integer) L3b != 0 ? (const integer) SAVE_EXPR LB0> : (const integer) SAVE_EXPR LB0>;
const integer R8b = (integer) L5b == 0 ? (const integer) SAVE_EXPR UB0> : (const integer) (((integer) L5b + -1) + (integer) L6b);
typedef p__TTS7bSP1___XD p__TTS7bSP1___XD;
typedef struct ;
typedef character p__TS7bS[(size_type) L6b:(integer) R8b >= (integer) L6b ? (size_type) R8b : (size_type) L6b + -1];
character S7b[(size_type) L6b:(integer) R8b >= (integer) L6b ? (size_type) R8b : (size_type) L6b + -1];
typedef struct ;
typedef character p__T9b[1:4];
typedef p__TS2bP1___XD p__TS2bP1___XD;
typedef struct ;
typedef character p__S2b[(size_type) SAVE_EXPR LB0>:SAVE_EXPR UB0> >= SAVE_EXPR LB0> ? (size_type) SAVE_EXPR UB0> : (size_type) SAVE_EXPR LB0> + -1];
typedef p__TS1bP1___XD p__TS1bP1___XD;
typedef struct ;
typedef character p__S1b[(size_type) SAVE_EXPR LB0>:SAVE_EXPR UB0> >= SAVE_EXPR LB0> ? (size_type) SAVE_EXPR UB0> : (size_type) SAVE_EXPR LB0> + -1];
typedef struct ;
typedef character p__T9b[1:4];
if ((boolean) b && (SAVE_EXPR >= SAVE_EXPR LB0> ? (SAVE_EXPR UB0> - SAVE_EXPR LB0>) + 1 : 0>);
const integer L4b = (const integer) (SAVE_EXPR UB0> >= SAVE_EXPR LB0> ? (SAVE_EXPR UB0> - SAVE_EXPR LB0>) + 1 : 0>);
const integer L5b = (const integer) ((integer) L3b + (integer) L4b);
const integer L6b = (integer) L3b != 0 ? (const integer) SAVE_EXPR LB0> : (const integer) SAVE_EXPR LB0>;
const integer R8b = (integer) L5b == 0 ? (const integer) SAVE_EXPR UB0> : (const integer) (((integer) L5b + -1) + (integer) L6b);
typedef p__TTS7bSP1___XD p__TTS7bSP1___XD;
typedef struct ;
typedef character p__TS7bS[(size_type) L6b:(integer) R8b >= (integer) L6b ? (size_type) R8b : (size_type) L6b + -1];
character S7b[(size_type) L6b:(integer) R8b >= (integer) L6b ? (size_type) R8b : (size_type) L6b + -1];
system.concat_2.str_concat_2 ({.P_ARRAY=(character[(size_type) .P_BOUNDS->LB0:.P_BOUNDS->UB0 >= .P_BOUNDS->LB0 ? (size_type) .P_BOUNDS->UB0 : (size_type) .P_BOUNDS->LB0 + -1] *) &S7b, .P_BOUNDS=&{.LB0=(integer) L6b, .UB0=(integer) R8b}}, s1, s2);>;, (integer) R8b - (integer) L6b == 3 && VIEW_CONVERT_EXPR(S7b) == "toto";))
{
.gnat_rcheck_21 ("p.adb", 4);
}
else
{
}
return;
}
_GLOBAL.SZ0.ada_p (integer p0, integer p1)
{
return p1 <= p0 ? (bit_size_type) size_type) p0 - (size_type) p1) + 1) * 8) : 0;
_GLOBAL.SZ1.ada_p (integer p0, integer p1)
{
return p1 <= p0 ? ((size_type) p0 - (size_type) p1) + 1 : 0;
Re: [patch] Couple of tweaks to the gimplifier
On Mon, Mar 21, 2011 at 12:19 PM, Eric Botcazou wrote: > Hi, > > the attached patch makes a couple of tweaks to the gimplifier in order to help > Ada, but I think that they are of general usefulness: > > 1) Set TREE_THIS_NOTRAP on the INDIRECT_REF built for VLA decls. This is > correct since stack memory isn't considered as trapping in the IL. This is ok. > 2) Improve gimplification of complex conditions in COND_EXPR. They are > naturally generated by the Ada compiler and the patch avoids emitting > redundant branches in GIMPLE, visible at -O0 for the testcase: Shouldn't + /* Remove any COMPOUND_EXPR so the following cases will be caught. */ + STRIP_TYPE_NOPS (TREE_OPERAND (expr, 0)); + if (TREE_CODE (TREE_OPERAND (expr, 0)) == COMPOUND_EXPR) +gimplify_compound_expr (&TREE_OPERAND (expr, 0), pre_p, true); happen in gimple_boolify instead so that other callers also benefit? That is, add a COMPOUND_EXPR case there? > procedure P (B : Boolean; S1, S2 : String) is > begin > if B and then S1 & S2 = "toto" then > raise Program_Error; > end if; > end; So, what does the GENERIC look like here? Thanks, Richard. > @@ -158,21 +158,12 @@ > movl %r12d, %eax > subl %ebx, %eax > cmpl $3, %eax > - jne .L33 > + jne .L18 > .loc 1 3 0 discriminator 1 > movq -40(%rbp), %rax > movl (%rax), %eax > cmpl $1869901684, %eax > - jne .L33 > - .loc 1 3 0 discriminator 2 > - movl $1, %eax > - jmp .L34 > -.L33: > - movl $0, %eax > -.L34: > - .loc 1 3 0 discriminator 3 > - testb %al, %al > - je .L18 > + jne .L18 > .loc 1 4 0 is_stmt 1 > movl $4, %esi > movl $.LC0, %edi > > Bootstrapped/regtested on x86_64-suse-linux, OK for the mainline? > > > 2011-03-21 Eric Botcazou > > * gimplify.c (gimplify_vla_decl): Set TREE_THIS_NOTRAP flag. > (gimplify_cond_expr): Gimplify COMPOUND_EXPR conditions. > > > -- > Eric Botcazou >
